Custom fields: track the changes and improve the robustness with regards to the Exception thrown by the handler. Also fixed an issue with DBObject, causing the custom fields to be written several times if invoking DBUpdate more than once. Theoretically, this issue affects any type of attribute.

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

core/attributedef.class.inc.php

@@ -6503,10 +6503,10 @@ class AttributeCustomFields extends AttributeDefinition
 	 * @param array|null $aValues
 	 * @return CustomFieldsHandler
 	 */
-	public function GetHandler(DBObject $oHostObject, $aValues = null)
+	public function GetHandler($aValues = null)
 	{
 		$sHandlerClass = $this->Get('handler_class');
-		$oHandler = new $sHandlerClass($oHostObject, $this->GetCode());
+		$oHandler = new $sHandlerClass($this->GetCode());
 		if (!is_null($aValues))
 		{
 			$oHandler->SetCurrentValues($aValues);
@@ -6581,11 +6581,23 @@ class AttributeCustomFields extends AttributeDefinition
 	 */
 	public function GetForm(DBObject $oHostObject, $sFormPrefix = null)
 	{
-		$oValue = $oHostObject->Get($this->GetCode());
-		$oHandler = $this->GetHandler($oHostObject, $oValue->GetValues());
-		$sFormId = is_null($sFormPrefix) ? 'cf_'.$this->GetCode() : $sFormPrefix.'_cf_'.$this->GetCode();
-		$oHandler->BuildForm($sFormId);
-		return $oHandler->GetForm();
+		try
+		{
+			$oValue = $oHostObject->Get($this->GetCode());
+			$oHandler = $this->GetHandler($oValue->GetValues());
+			$sFormId = is_null($sFormPrefix) ? 'cf_'.$this->GetCode() : $sFormPrefix.'_cf_'.$this->GetCode();
+			$oHandler->BuildForm($oHostObject, $sFormId);
+			$oForm = $oHandler->GetForm();
+		}
+		catch (Exception $e)
+		{
+			$oForm = new \Combodo\iTop\Form\Form('');
+			$oField = new \Combodo\iTop\Form\Field\LabelField('');
+			$oField->SetLabel('Custom field error: '.$e->getMessage());
+			$oForm->AddField($oField);
+			$oForm->Finalize();
+		}
+		return $oForm;
 	}
 
 	/**
@@ -6595,9 +6607,16 @@ class AttributeCustomFields extends AttributeDefinition
 	 */
 	public function ReadValue($oHostObject)
 	{
-		$oHandler = $this->GetHandler($oHostObject);
-		$aValues = $oHandler->ReadValues();
-		$oRet = new ormCustomFieldsValue($oHostObject, $this->GetCode(), $aValues);
+		try
+		{
+			$oHandler = $this->GetHandler();
+			$aValues = $oHandler->ReadValues($oHostObject);
+			$oRet = new ormCustomFieldsValue($oHostObject, $this->GetCode(), $aValues);
+		}
+		catch (Exception $e)
+		{
+			$oRet = new ormCustomFieldsValue($oHostObject, $this->GetCode());
+		}
 		return $oRet;
 	}
 
@@ -6611,18 +6630,18 @@ class AttributeCustomFields extends AttributeDefinition
 	{
 		if (is_null($oValue))
 		{
-			$oHandler = $this->GetHandler($oHostObject);
+			$oHandler = $this->GetHandler();
 			$aValues = array();
 		}
 		else
 		{
 			// Pass the values through the form to make sure that they are correct
-			$oHandler = $this->GetHandler($oHostObject, $oValue->GetValues());
-			$oHandler->BuildForm('');
+			$oHandler = $this->GetHandler($oValue->GetValues());
+			$oHandler->BuildForm($oHostObject, '');
 			$oForm = $oHandler->GetForm();
 			$aValues = $oForm->GetCurrentValues();
 		}
-		return $oHandler->WriteValues($aValues);
+		return $oHandler->WriteValues($oHostObject, $aValues);
 	}
 
 	/**
@@ -6635,8 +6654,8 @@ class AttributeCustomFields extends AttributeDefinition
 	{
 		try
 		{
-			$oHandler = $this->GetHandler($oHostObject, $value->GetValues());
-			$oHandler->BuildForm('');
+			$oHandler = $this->GetHandler($value->GetValues());
+			$oHandler->BuildForm($oHostObject, '');
 			$oForm = $oHandler->GetForm();
 			$oForm->Validate();
 			if ($oForm->GetValid())
@@ -6667,23 +6686,50 @@ class AttributeCustomFields extends AttributeDefinition
 	public function DeleteValue(DBObject $oHostObject)
 	{
 		$oValue = $oHostObject->Get($this->GetCode());
-		$oHandler = $this->GetHandler($oHostObject, $oValue->GetValues());
-		return $oHandler->DeleteValues();
+		$oHandler = $this->GetHandler($oValue->GetValues());
+		return $oHandler->DeleteValues($oHostObject);
 	}
 
 	public function GetAsHTML($value, $oHostObject = null, $bLocalize = true)
 	{
-		return $value->GetAsHTML($bLocalize);
+		try
+		{
+			$sRet = $value->GetAsHTML($bLocalize);
+		}
+		catch (Exception $e)
+		{
+			$sRet = 'Custom field error: '.htmlentities($e->getMessage(), ENT_QUOTES, 'UTF-8');
+		}
+		return $sRet;
 	}
 
 	public function GetAsXML($value, $oHostObject = null, $bLocalize = true)
 	{
-		return $value->GetAsXML($bLocalize);
+		try
+		{
+			$sRet = $value->GetAsXML($bLocalize);
+		}
+		catch (Exception $e)
+		{
+			$sRet = Str::pure2xml('Custom field error: '.$e->getMessage());
+		}
+		return $sRet;
 	}
 
 	public function GetAsCSV($value, $sSeparator = ',', $sTextQualifier = '"', $oHostObject = null, $bLocalize = true, $bConvertToPlainText = false)
 	{
-		return $value->GetAsCSV($sSeparator, $sTextQualifier, $bLocalize, $bConvertToPlainText);
+		try
+		{
+			$sRet = $value->GetAsCSV($sSeparator, $sTextQualifier, $bLocalize, $bConvertToPlainText);
+		}
+		catch (Exception $e)
+		{
+			$sFrom = array("\r\n", $sTextQualifier);
+			$sTo = array("\n", $sTextQualifier.$sTextQualifier);
+			$sEscaped = str_replace($sFrom, $sTo, 'Custom field error: '.$e->getMessage());
+			$sRet = $sTextQualifier.$sEscaped.$sTextQualifier;
+		}
+		return $sRet;
 	}
 
 	/**
@@ -6704,7 +6750,15 @@ class AttributeCustomFields extends AttributeDefinition
 	 */
 	public function GetForTemplate($value, $sVerb, $oHostObject = null, $bLocalize = true)
 	{
-		return $value->GetForTemplate($sVerb, $bLocalize);
+		try
+		{
+			$sRet = $value->GetForTemplate($sVerb, $bLocalize);
+		}
+		catch (Exception $e)
+		{
+			$sRet = 'Custom field error: '.$e->getMessage();
+		}
+		return $sRet;
 	}
 
 	public function MakeValueFromString($sProposedValue, $bLocalizedValue = false, $sSepItem = null, $sSepAttribute = null, $sSepValue = null, $sAttributeQualifier = null)
@@ -6732,7 +6786,15 @@ class AttributeCustomFields extends AttributeDefinition
 
 	public function Equals($val1, $val2)
 	{
-		return $val1->Equals($val2);
+		try
+		{
+			$bEquals = $val1->Equals($val2);
+		}
+		catch (Exception $e)
+		{
+			false;
+		}
+		return $bEquals;
 	}
 }
 

core/cmdbchangeop.class.inc.php

@@ -910,4 +910,70 @@ class CMDBChangeOpSetAttributeLinksTune extends CMDBChangeOpSetAttributeLinks
 		return $sResult;
 	}
 }
-?>
+
+/**
+ * Record the modification of custom fields
+ *
+ * @package     iTopORM
+ */
+class CMDBChangeOpSetAttributeCustomFields extends CMDBChangeOpSetAttribute
+{
+	public static function Init()
+	{
+		$aParams = array
+		(
+			"category" => "core/cmdb",
+			"key_type" => "",
+			"name_attcode" => "change",
+			"state_attcode" => "",
+			"reconc_keys" => array(),
+			"db_table" => "priv_changeop_setatt_custfields",
+			"db_key_field" => "id",
+			"db_finalclass_field" => "",
+		);
+		MetaModel::Init_Params($aParams);
+		MetaModel::Init_InheritAttributes();
+		MetaModel::Init_AddAttribute(new AttributeLongText("prevdata", array("allowed_values"=>null, "sql"=>"prevdata", "default_value"=>"", "is_null_allowed"=>true, "depends_on"=>array())));
+
+		// Display lists
+		MetaModel::Init_SetZListItems('details', array('date', 'userinfo', 'attcode')); // Attributes to be displayed for the complete details
+		MetaModel::Init_SetZListItems('list', array('date', 'userinfo', 'attcode')); // Attributes to be displayed for a list
+	}
+
+	/**
+	 * Describe (as a text string) the modifications corresponding to this change
+	 */
+	public function GetDescription()
+	{
+		$sResult = '';
+		if (MetaModel::IsValidAttCode($this->Get('objclass'), $this->Get('attcode')))
+		{
+			$oTargetObjectClass = $this->Get('objclass');
+			$oTargetObjectKey = $this->Get('objkey');
+			$oTargetSearch = new DBObjectSearch($oTargetObjectClass);
+			$oTargetSearch->AddCondition('id', $oTargetObjectKey, '=');
+
+			$oMonoObjectSet = new DBObjectSet($oTargetSearch);
+			if (UserRights::IsActionAllowedOnAttribute($this->Get('objclass'), $this->Get('attcode'), UR_ACTION_READ, $oMonoObjectSet) == UR_ALLOWED_YES)
+			{
+				$aValues = json_decode($this->Get('prevdata'), true);
+				$oAttDef = MetaModel::GetAttributeDef($this->Get('objclass'), $this->Get('attcode'));
+				$sAttName = $oAttDef->GetLabel();
+
+				try
+				{
+					$oHandler = $oAttDef->GetHandler($aValues);
+					$sValueDesc = $oHandler->GetAsHTML($aValues);
+				}
+				catch (Exception $e)
+				{
+					$sValueDesc = 'Custom field error: '.htmlentities($e->getMessage(), ENT_QUOTES, 'UTF-8');
+				}
+				$sTextView = '<div>'.$sValueDesc.'</div>';
+
+				$sResult = Dict::Format('Change:AttName_Changed_PreviousValue_OldValue', $sAttName, $sTextView);
+			}
+		}
+		return $sResult;
+	}
+}

core/cmdbobject.class.inc.php

@@ -396,6 +396,17 @@ abstract class CMDBObject extends DBObject
 				$oMyChangeOp->Set("newvalue", $value[$sAttCode]);
 				$iId = $oMyChangeOp->DBInsertNoReload();
 			}
+			elseif ($oAttDef instanceOf AttributeCustomFields)
+			{
+				// Custom fields
+				//
+				$oMyChangeOp = MetaModel::NewObject("CMDBChangeOpSetAttributeCustomFields");
+				$oMyChangeOp->Set("objclass", get_class($this));
+				$oMyChangeOp->Set("objkey", $this->GetKey());
+				$oMyChangeOp->Set("attcode", $sAttCode);
+				$oMyChangeOp->Set("prevdata", json_encode($original->GetValues()));
+				$iId = $oMyChangeOp->DBInsertNoReload();
+			}
 			else
 			{
 				// Scalars
@@ -536,13 +547,13 @@ abstract class CMDBObject extends DBObject
 
 	public static function BulkUpdate(DBSearch $oFilter, array $aValues)
 	{
-		return $this->BulkUpdateTracked_Internal($oFilter, $aValues);
+		return static::BulkUpdateTracked_Internal($oFilter, $aValues);
 	}
 
 	public static function BulkUpdateTracked(CMDBChange $oChange, DBSearch $oFilter, array $aValues)
 	{
 		self::SetCurrentChange($oChange);
-		$this->BulkUpdateTracked_Internal($oFilter, $aValues);
+		static::BulkUpdateTracked_Internal($oFilter, $aValues);
 	}
 
 	protected static function BulkUpdateTracked_Internal(DBSearch $oFilter, array $aValues)

core/customfieldshandler.class.inc.php

@@ -28,7 +28,6 @@ use Combodo\iTop\Form\FormManager;
 
 abstract class CustomFieldsHandler
 {
-	protected $oHostObject;
 	protected $sAttCode;
 	protected $aValues;
 	protected $oForm;
@@ -37,17 +36,15 @@ abstract class CustomFieldsHandler
 	 * This constructor's prototype must be frozen.
 	 * Any specific behavior must be implemented in BuildForm()
 	 *
-	 * @param DBObject $oHostObject
 	 * @param $sAttCode
 	 */
-	final public function __construct(DBObject $oHostObject, $sAttCode)
+	final public function __construct($sAttCode)
 	{
-		$this->oHostObject = $oHostObject;
 		$this->sAttCode = $sAttCode;
 		$this->aValues = null;
 	}
 
-	abstract public function BuildForm($sFormId);
+	abstract public function BuildForm(DBObject $oHostObject, $sFormId);
 
 	/**
 	 *
@@ -109,21 +106,24 @@ abstract class CustomFieldsHandler
 	abstract public function GetAsCSV($aValues, $sSeparator = ',', $sTextQualifier = '"', $bLocalize = true);
 
 	/**
+	 * @param DBObject $oHostObject
 	 * @return array Associative array id => value
 	 */
-	abstract public function ReadValues();
+	abstract public function ReadValues(DBObject $oHostObject);
 
 	/**
 	 * Record the data (currently in the processing of recording the host object)
 	 * It is assumed that the data has been checked prior to calling Write()
+	 * @param DBObject $oHostObject
 	 * @param array Associative array id => value
 	 */
-	abstract public function WriteValues($aValues);
+	abstract public function WriteValues(DBObject $oHostObject, $aValues);
 
 	/**
 	 * Cleanup data upon object deletion (object id still available here)
+	 * @param DBObject $oHostObject
 	 */
-	abstract public function DeleteValues();
+	abstract public function DeleteValues(DBObject $oHostObject);
 
 	/**
 	 * @param $aValuesA

core/dbobject.class.php

@@ -1428,6 +1428,8 @@ abstract class DBObject implements iDisplay
 		{
 			if (!$oAttDef->LoadInObject()) continue;
 			if ($oAttDef->LoadFromDB()) continue;
+			if (!array_key_exists($sAttCode, $this->m_aTouchedAtt)) continue;
+			if (array_key_exists($sAttCode, $this->m_aModifiedAtt) && ($this->m_aModifiedAtt[$sAttCode] == false)) continue;
 			$oAttDef->WriteValue($this, $this->m_aCurrValues[$sAttCode]);
 		}
 	}
@@ -1841,11 +1843,15 @@ abstract class DBObject implements iDisplay
 
 			$bHasANewExternalKeyValue = false;
 			$aHierarchicalKeys = array();
+			$aDBChanges = array();
 			foreach($aChanges as $sAttCode => $valuecurr)
 			{
 				$oAttDef = MetaModel::GetAttributeDef(get_class($this), $sAttCode);
 				if ($oAttDef->IsExternalKey()) $bHasANewExternalKeyValue = true;
-				if (!$oAttDef->IsDirectField()) unset($aChanges[$sAttCode]);
+				if ($oAttDef->IsDirectField())
+				{
+					$aDBChanges[$sAttCode] = $aChanges[$sAttCode];
+				}
 				if ($oAttDef->IsHierarchicalKey())
 				{
 					$aHierarchicalKeys[$sAttCode] = $oAttDef;
@@ -1865,7 +1871,7 @@ abstract class DBObject implements iDisplay
 					$iDelta =$iMyRight - $iMyLeft + 1;
 					MetaModel::HKTemporaryCutBranch($iMyLeft, $iMyRight, $oAttDef, $sTable);
 					
-					if ($aChanges[$sAttCode] == 0)
+					if ($aDBChanges[$sAttCode] == 0)
 					{
 						// No new parent, insert completely at the right of the tree
 						$sSQL = "SELECT max(`".$oAttDef->GetSQLRight()."`) AS max FROM `$sTable`";
@@ -1882,33 +1888,36 @@ abstract class DBObject implements iDisplay
 					else
 					{
 						// Insert at the right of the specified parent
-						$sSQL = "SELECT `".$oAttDef->GetSQLRight()."` FROM `$sTable` WHERE id=".((int)$aChanges[$sAttCode]);
+						$sSQL = "SELECT `".$oAttDef->GetSQLRight()."` FROM `$sTable` WHERE id=".((int)$aDBChanges[$sAttCode]);
 						$iNewLeft = CMDBSource::QueryToScalar($sSQL);
 					}
 
 					MetaModel::HKReplugBranch($iNewLeft, $iNewLeft + $iDelta - 1, $oAttDef, $sTable);
 
 					$aHKChanges = array();
-					$aHKChanges[$sAttCode] = $aChanges[$sAttCode];
+					$aHKChanges[$sAttCode] = $aDBChanges[$sAttCode];
 					$aHKChanges[$oAttDef->GetSQLLeft()] = $iNewLeft;
 					$aHKChanges[$oAttDef->GetSQLRight()] = $iNewLeft + $iDelta - 1;
-					$aChanges[$sAttCode] = $aHKChanges; // the 3 values will be stored by MakeUpdateQuery below
+					$aDBChanges[$sAttCode] = $aHKChanges; // the 3 values will be stored by MakeUpdateQuery below
 				}
 				
 				// Update scalar attributes
-				if (count($aChanges) != 0)
+				if (count($aDBChanges) != 0)
 				{
 					$oFilter = new DBObjectSearch(get_class($this));
 					$oFilter->AddCondition('id', $this->m_iKey, '=');
 			
-					$sSQL = $oFilter->MakeUpdateQuery($aChanges);
+					$sSQL = $oFilter->MakeUpdateQuery($aDBChanges);
 					CMDBSource::Query($sSQL);
 				}
 			}
 
 			$this->DBWriteLinks();
 			$this->WriteExternalAttributes();
+
 			$this->m_bDirty = false;
+			$this->m_aTouchedAtt = array();
+			$this->m_aModifiedAtt = array();
 
 			$this->AfterUpdate();
 
@@ -1928,7 +1937,6 @@ abstract class DBObject implements iDisplay
 						$this->m_aOrigValues[$sAttCode] = is_object($value) ? clone $value : $value;
 					}
 				}
-			
 			}
 
 			if (count($aChanges) != 0)

core/ormcustomfieldsvalue.class.inc.php

@@ -59,21 +59,21 @@ class ormCustomFieldsValue
 	public function GetAsHTML($bLocalize = true)
 	{
 		$oAttDef = MetaModel::GetAttributeDef(get_class($this->oHostObject), $this->sAttCode);
-		$oHandler = $oAttDef->GetHandler($this->oHostObject, $this->GetValues());
+		$oHandler = $oAttDef->GetHandler($this->GetValues());
 		return $oHandler->GetAsHTML($this->aCurrentValues, $bLocalize);
 	}
 
 	public function GetAsXML($bLocalize = true)
 	{
 		$oAttDef = MetaModel::GetAttributeDef(get_class($this->oHostObject), $this->sAttCode);
-		$oHandler = $oAttDef->GetHandler($this->oHostObject, $this->GetValues());
+		$oHandler = $oAttDef->GetHandler($this->GetValues());
 		return $oHandler->GetAsXML($this->aCurrentValues, $bLocalize);
 	}
 
 	public function GetAsCSV($sSeparator = ',', $sTextQualifier = '"', $bLocalize = true)
 	{
 		$oAttDef = MetaModel::GetAttributeDef(get_class($this->oHostObject), $this->sAttCode);
-		$oHandler = $oAttDef->GetHandler($this->oHostObject, $this->GetValues());
+		$oHandler = $oAttDef->GetHandler($this->GetValues());
 		return $oHandler->GetAsCSV($this->aCurrentValues, $sSeparator = ',', $sTextQualifier = '"', $bLocalize = true);
 	}
 
@@ -86,7 +86,7 @@ class ormCustomFieldsValue
 	public function GetForTemplate($sVerb, $bLocalize = true)
 	{
 		$oAttDef = MetaModel::GetAttributeDef(get_class($this->oHostObject), $this->sAttCode);
-		$oHandler = $oAttDef->GetHandler($this->oHostObject, $this->GetValues());
+		$oHandler = $oAttDef->GetHandler($this->GetValues());
 		return 'template...verb='.$sVerb.' sur "'.json_encode($this->aCurrentValues).'"';
 	}
 
@@ -97,7 +97,7 @@ class ormCustomFieldsValue
 	public function Equals(ormCustomFieldsValue $oReference)
 	{
 		$oAttDef = MetaModel::GetAttributeDef(get_class($this->oHostObject), $this->sAttCode);
-		$oHandler = $oAttDef->GetHandler($this->oHostObject, $this->GetValues());
+		$oHandler = $oAttDef->GetHandler($this->GetValues());
 		return $oHandler->CompareValues($this->aCurrentValues, $oReference->aCurrentValues);
 	}
 }