Procházet zdrojové kódy

Rework of the edition of 1-N and N-N links: managed as a delta from the GUI down to the the lowest APIs.
- Fixes the management of obsolete linked data.
- N.744 Fixes concurrent modifications (example: a user modifies a team, another user modifies a person related to that same team). Still NOT fixed with the customer portal.
- N.849 Fixes links edition in the case some data are not allowed to the current user (organization silos) -TO BE TESTED
- #1145 Fixes the creation of duplicate links in one step (Server to NW Device)
- #1147 Fixes the update of duplicate links

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

romainq před 8 roky
rodič
revize
0e1f3d02b9

+ 85 - 112
application/cmdbabstract.class.inc.php

@@ -405,11 +405,28 @@ EOF
 			$oAttDef = MetaModel::GetAttributeDef(get_class($this), $sAttCode);
 			// Display mode
 			if (!$oAttDef->IsLinkset()) continue; // Process only linkset attributes...
-			
-			// $oSet = new DBObjectSet($this->Get($sAttCode)->GetFilter()); // Why do something so useless ?
-			$oSet = $this->Get($sAttCode);
-			$oSet->SetShowObsoleteData(utils::ShowObsoleteData());
-			$iCount = $oSet->Count();
+
+			$sLinkedClass = $oAttDef->GetLinkedClass();
+
+			// Filter out links pointing to obsolete objects (if relevant)
+			$oLinkSearch = $this->Get($sAttCode)->GetFilter();
+			if ($oAttDef->IsIndirect())
+			{
+				$oLinkingAttDef = MetaModel::GetAttributeDef($sLinkedClass, $oAttDef->GetExtKeyToRemote());
+				$sTargetClass = $oLinkingAttDef->GetTargetClass();
+				if (!utils::ShowObsoleteData() && MetaModel::IsObsoletable($sTargetClass))
+				{
+					$oNotObsolete = new BinaryExpression(
+						new FieldExpression('obsolescence_flag', $sTargetClass),
+						'=',
+						new ScalarExpression(0)
+					);
+					$oLinkSearch->AddConditionExpression($oNotObsolete);
+				}
+			}
+			$oLinkSet = new DBObjectSet($oLinkSearch);
+
+			$iCount = $oLinkSet->Count();
 			$sCount = '';
 			if ($iCount != 0)
 			{
@@ -427,8 +444,7 @@ EOF
 			// Adjust the flags according to user rights
 			if ($oAttDef->IsIndirect())
 			{
-				$sLinkedClass = $oAttDef->GetLinkedClass();
-				$oLinkingAttDef = 	MetaModel::GetAttributeDef($sLinkedClass, $oAttDef->GetExtKeyToRemote());
+				$oLinkingAttDef = MetaModel::GetAttributeDef($sLinkedClass, $oAttDef->GetExtKeyToRemote());
 				$sTargetClass = $oLinkingAttDef->GetTargetClass();
 				// n:n links => must be allowed to modify the linking class AND  read the target class in order to edit the linkedset
 				if (!UserRights::IsActionAllowed($sLinkedClass, UR_ACTION_MODIFY) || !UserRights::IsActionAllowed($sTargetClass, UR_ACTION_READ))
@@ -444,12 +460,12 @@ EOF
 			else
 			{
 				// 1:n links => must be allowed to modify the linked class in order to edit the linkedset
-				if (!UserRights::IsActionAllowed($oAttDef->GetLinkedClass(), UR_ACTION_MODIFY))
+				if (!UserRights::IsActionAllowed($sLinkedClass, UR_ACTION_MODIFY))
 				{
 					$iFlags |= OPT_ATT_READONLY;
 				}
 				// 1:n links => must be allowed to read the linked class in order to display the linkedset
-				if (!UserRights::IsActionAllowed($oAttDef->GetLinkedClass(), UR_ACTION_READ))
+				if (!UserRights::IsActionAllowed($sLinkedClass, UR_ACTION_READ))
 				{
 					$iFlags |= OPT_ATT_HIDDEN;
 				}
@@ -463,10 +479,9 @@ EOF
 			{
 				$sInputId = $this->m_iFormId.'_'.$sAttCode;
 
-				$sLinkedClass = $oAttDef->GetLinkedClass();
 				if ($oAttDef->IsIndirect())
 				{
-					$oLinkingAttDef = 	MetaModel::GetAttributeDef($sLinkedClass, $oAttDef->GetExtKeyToRemote());
+					$oLinkingAttDef = MetaModel::GetAttributeDef($sLinkedClass, $oAttDef->GetExtKeyToRemote());
 					$sTargetClass = $oLinkingAttDef->GetTargetClass();
 				}
 				else
@@ -475,9 +490,8 @@ EOF
 				}
 				$oPage->p(MetaModel::GetClassIcon($sTargetClass)."&nbsp;".$oAttDef->GetDescription().'<span id="busy_'.$sInputId.'"></span>');
 
-				$oValue = $this->Get($sAttCode);
 				$sDisplayValue = ''; // not used
-				$sHTMLValue = "<span id=\"field_{$sInputId}\">".self::GetFormElementForField($oPage, $sClass, $sAttCode, $oAttDef, $oValue, $sDisplayValue, $sInputId, '', $iFlags, $aArgs).'</span>';
+				$sHTMLValue = "<span id=\"field_{$sInputId}\">".self::GetFormElementForField($oPage, $sClass, $sAttCode, $oAttDef, $oLinkSet, $sDisplayValue, $sInputId, '', $iFlags, $aArgs).'</span>';
 				$this->AddToFieldsMap($sAttCode,  $sInputId);
 				$oPage->add($sHTMLValue);
 			}
@@ -487,7 +501,7 @@ EOF
 				if (!$oAttDef->IsIndirect())
 				{
 					// 1:n links
-					$sTargetClass = $oAttDef->GetLinkedClass();
+					$sTargetClass = $sLinkedClass;
 
 					$aDefaults = array($oAttDef->GetExtKeyToMe() => $this->GetKey());
 					$oAppContext = new ApplicationContext();
@@ -510,10 +524,8 @@ EOF
 				else
 				{
 					// n:n links
-					$sLinkedClass = $oAttDef->GetLinkedClass();
-					$oLinkingAttDef = 	MetaModel::GetAttributeDef($sLinkedClass, $oAttDef->GetExtKeyToRemote());
+					$oLinkingAttDef = MetaModel::GetAttributeDef($sLinkedClass, $oAttDef->GetExtKeyToRemote());
 					$sTargetClass = $oLinkingAttDef->GetTargetClass();
-					$bMenu = ($this->Get($sAttCode)->Count() > 0); // The menu is enabled only if there are already some elements...
 					$aParams = array(
 							'link_attr' => $oAttDef->GetExtKeyToMe(),
 							'object_id' => $this->GetKey(),
@@ -525,7 +537,7 @@ EOF
 						);
 				}
 				$oPage->p(MetaModel::GetClassIcon($sTargetClass)."&nbsp;".$oAttDef->GetDescription());
-				$oBlock = new DisplayBlock($this->Get($sAttCode)->GetFilter(), 'list', false);
+				$oBlock = new DisplayBlock($oLinkSet->GetFilter(), 'list', false);
 				$oBlock->Display($oPage, 'rel_'.$sAttCode, $aParams);
 			}
 			if (array_key_exists($sAttCode, $aRedundancySettings))
@@ -2107,7 +2119,7 @@ EOF
 		$oPage->add_dict_entry('UI:ValueMustBeSet');
 		$oPage->add_dict_entry('UI:ValueMustBeChanged');
 		$oPage->add_dict_entry('UI:ValueInvalidFormat');
-		return "<div>{$sHTMLValue}</div>";
+		return "<div class=\"attribute-edit\" data-attcode=\"$sAttCode\">{$sHTMLValue}</div>";
 	}
 
 	public function DisplayModifyForm(WebPage $oPage, $aExtraParams = array())
@@ -3026,50 +3038,7 @@ EOF
 		foreach($aValues as $sAttCode => $value)
 		{
 			$oAttDef = MetaModel::GetAttributeDef(get_class($this), $sAttCode);
-			if ($oAttDef->IsLinkSet() && $oAttDef->IsIndirect())
-			{
-				$aLinks = $value;
-				$sLinkedClass = $oAttDef->GetLinkedClass();
-				$sExtKeyToRemote = $oAttDef->GetExtKeyToRemote();
-				$sExtKeyToMe = $oAttDef->GetExtKeyToMe();
-				$oLinkedSet = DBObjectSet::FromScratch($sLinkedClass);
-				if (is_array($aLinks))
-				{
-					foreach($aLinks as $id => $aData)
-					{
-						if (is_numeric($id))
-						{
-							if ($id < 0)
-							{
-								// New link to be created, the opposite of the id (-$id) is the ID of the remote object
-								$oLink = MetaModel::NewObject($sLinkedClass);
-								$oLink->Set($sExtKeyToRemote, -$id);
-								$oLink->Set($sExtKeyToMe, $this->GetKey());
-							}
-							else
-							{
-								// Existing link, potentially to be updated...
-								$oLink = MetaModel::GetObject($sLinkedClass, $id);
-							}
-							// Now populate the attributes
-							foreach($aData as $sName => $value)
-							{
-								if (MetaModel::IsValidAttCode($sLinkedClass, $sName))
-								{
-									$oLinkAttDef = MetaModel::GetAttributeDef($sLinkedClass, $sName);
-									if ($oLinkAttDef->IsWritable())
-									{
-										$oLink->Set($sName, $value);
-									}
-								}
-							}
-							$oLinkedSet->AddObject($oLink);
-						}
-					}
-				}
-				$this->Set($sAttCode, $oLinkedSet);
-			}
-			elseif ($oAttDef->GetEditClass() == 'Document')
+			if ($oAttDef->GetEditClass() == 'Document')
 			{
 				// There should be an uploaded file with the named attr_<attCode>
 				$oDocument = $value['fcontents'];
@@ -3123,30 +3092,10 @@ EOF
 			{
 				$this->Set($sAttCode, $value);
 			}
-			else if (($oAttDef->GetEditClass() == 'LinkedSet') && !$oAttDef->IsIndirect() &&
-			          (($oAttDef->GetEditMode() == LINKSET_EDITMODE_INPLACE) || ($oAttDef->GetEditMode() == LINKSET_EDITMODE_ADDREMOVE)))
+			else if ($oAttDef->GetEditClass() == 'LinkedSet')
 			{
-				$oLinkset = $this->Get($sAttCode);
-				$sLinkedClass = $oLinkset->GetClass();
-				$aObjSet = array();
-				$oLinkset->Rewind();
-				$bModified = false;
-				while($oLink = $oLinkset->Fetch())
-				{
-					if (in_array($oLink->GetKey(), $value['to_be_deleted']))
-					{
-						// The link is to be deleted, don't copy it in the array
-						$bModified = true;
-					}
-					else
-					{
-						if (!array_key_exists('to_be_removed', $value) || !in_array($oLink->GetKey(), $value['to_be_removed']))
-						{
-							$aObjSet[] = $oLink;
-						}
-					}
-				}
-
+				$oLinkSet = $this->Get($sAttCode);
+				$sLinkedClass = $oAttDef->GetLinkedClass();
 				if (array_key_exists('to_be_created', $value) && (count($value['to_be_created']) > 0))
 				{
 					// Now handle the links to be created
@@ -3156,11 +3105,10 @@ EOF
 						if ( ($sLinkedClass == $sSubClass) || (is_subclass_of($sSubClass, $sLinkedClass)) )
 						{
 							$aObjData = $aData['data'];
-							
-							$oLink = new $sSubClass;
+
+							$oLink = MetaModel::NewObject($sSubClass);
 							$oLink->UpdateObjectFromArray($aObjData);
-							$aObjSet[] = $oLink;
-							$bModified = true;
+							$oLinkSet->AddItem($oLink);
 						}
 					}
 				}
@@ -3172,32 +3120,39 @@ EOF
 						$oLink = MetaModel::GetObject($sLinkedClass, $iObjKey, false);
 						if ($oLink)
 						{
-							$aObjSet[] = $oLink;
-							$bModified = true;
+							$oLinkSet->AddItem($oLink);
 						}
 					}
 				}
-				if (array_key_exists('to_be_removed', $value) && (count($value['to_be_removed']) > 0))
+				if (array_key_exists('to_be_modified', $value) && (count($value['to_be_modified']) > 0))
 				{
-					// Now handle the links to be removed by making the remote object point to nothing
-					// Keep them in the set (modified), DBWriteLinks will handle them
-					foreach($value['to_be_removed'] as $iObjKey)
+					// Now handle the links to be added by making the remote object point to self
+					foreach($value['to_be_modified'] as $iObjKey => $aData)
 					{
 						$oLink = MetaModel::GetObject($sLinkedClass, $iObjKey, false);
 						if ($oLink)
 						{
-							$sExtKeyToMe = $oAttDef->GetExtKeyToMe();
-							$oLink->Set($sExtKeyToMe, null);
-							$aObjSet[] = $oLink;
-							$bModified = true;
+							$aObjData = $aData['data'];
+							$oLink->UpdateObjectFromArray($aObjData);
+							$oLinkSet->ModifyItem($oLink);
 						}
 					}
 				}
-				if ($bModified)
+				if (array_key_exists('to_be_removed', $value) && (count($value['to_be_removed']) > 0))
 				{
-					$oNewSet = DBObjectSet::FromArray($oLinkset->GetClass(), $aObjSet);
-					$this->Set($sAttCode, $oNewSet);
-				}		
+					foreach($value['to_be_removed'] as $iObjKey)
+					{
+						$oLinkSet->RemoveItem($iObjKey);
+					}
+				}
+				if (array_key_exists('to_be_deleted', $value) && (count($value['to_be_deleted']) > 0))
+				{
+					foreach($value['to_be_deleted'] as $iObjKey)
+					{
+						$oLinkSet->RemoveItem($iObjKey);
+					}
+				}
+				$this->Set($sAttCode, $oLinkSet);
 			}
 			else
 			{
@@ -3254,15 +3209,14 @@ EOF
 			{
 				$value = $oAttDef->ReadValueFromPostedForm($this, $sFormPrefix);
 			}
-			else if (($oAttDef->GetEditClass() == 'LinkedSet') && !$oAttDef->IsIndirect() &&
-			         (($oAttDef->GetEditMode() == LINKSET_EDITMODE_INPLACE) || ($oAttDef->GetEditMode() == LINKSET_EDITMODE_ADDREMOVE)) )
+			else if ($oAttDef->GetEditClass() == 'LinkedSet')
 			{
 				$aRawToBeCreated = json_decode(utils::ReadPostedParam("attr_{$sFormPrefix}{$sAttCode}_tbc", '{}', 'raw_data'), true);
 				$aToBeCreated = array();
 				foreach($aRawToBeCreated as $aData)
 				{
 					$sSubFormPrefix = $aData['formPrefix'];
-					$sObjClass = $aData['class'];
+					$sObjClass = $oAttDef->GetLinkedClass();
 					$aObjData = array();
 					foreach($aData as $sKey => $value)
 					{
@@ -3273,11 +3227,30 @@ EOF
 					}
 					$aToBeCreated[] = array('class' => $sObjClass, 'data' => $aObjData);
 				}
-				
-				$value = array('to_be_created' => $aToBeCreated, 
-							   'to_be_deleted' => json_decode(utils::ReadPostedParam("attr_{$sFormPrefix}{$sAttCode}_tbd", '[]', 'raw_data'), true), 
-							   'to_be_added' => json_decode(utils::ReadPostedParam("attr_{$sFormPrefix}{$sAttCode}_tba", '[]', 'raw_data'), true),
-							   'to_be_removed' => json_decode(utils::ReadPostedParam("attr_{$sFormPrefix}{$sAttCode}_tbr", '[]', 'raw_data'), true) );
+
+				$aRawToBeModified = json_decode(utils::ReadPostedParam("attr_{$sFormPrefix}{$sAttCode}_tbm", '{}', 'raw_data'), true);
+				$aToBeModified = array();
+				foreach($aRawToBeModified as $iObjKey => $aData)
+				{
+					$sSubFormPrefix = $aData['formPrefix'];
+					$aObjData = array();
+					foreach($aData as $sKey => $value)
+					{
+						if (preg_match("/^attr_$sSubFormPrefix(.*)$/", $sKey, $aMatches))
+						{
+							$aObjData[$aMatches[1]] = $value;
+						}
+					}
+					$aToBeModified[$iObjKey] = array('data' => $aObjData);
+				}
+
+				$value = array(
+					'to_be_created' => $aToBeCreated,
+					'to_be_modified' => $aToBeModified,
+					'to_be_deleted' => json_decode(utils::ReadPostedParam("attr_{$sFormPrefix}{$sAttCode}_tbd", '[]', 'raw_data'), true),
+					'to_be_added' => json_decode(utils::ReadPostedParam("attr_{$sFormPrefix}{$sAttCode}_tba", '[]', 'raw_data'), true),
+					'to_be_removed' => json_decode(utils::ReadPostedParam("attr_{$sFormPrefix}{$sAttCode}_tbr", '[]', 'raw_data'), true)
+				);
 			}
 			else if ($oAttDef instanceof AttributeDateTime) // AttributeDate is derived from AttributeDateTime
 			{

+ 28 - 5
application/ui.linksdirectwidget.class.inc.php

@@ -1,5 +1,5 @@
 <?php
-// Copyright (C) 2010-2015 Combodo SARL
+// Copyright (C) 2010-2017 Combodo SARL
 //
 //   This file is part of iTop.
 //
@@ -19,7 +19,7 @@
 /**
  * Class UILinksWidgetDirect
  *  
- * @copyright   Copyright (C) 2010-2015 Combodo SARL
+ * @copyright   Copyright (C) 2010-2017 Combodo SARL
  * @license     http://opensource.org/licenses/AGPL-3.0
  */ 
 
@@ -71,7 +71,14 @@ class UILinksWidgetDirect
 		}
 		
 	}
-	
+
+	/**
+	 * @param WebPage $oPage
+	 * @param DBObjectSet $oValue
+	 * @param array $aArgs
+	 * @param $sFormPrefix
+	 * @param $oCurrentObj
+	 */
 	public function Display(WebPage $oPage, DBObjectSet $oValue, $aArgs = array(), $sFormPrefix, $oCurrentObj)
 	{
 		$oLinksetDef = MetaModel::GetAttributeDef($this->sClass, $this->sAttCode);
@@ -115,7 +122,15 @@ class UILinksWidgetDirect
 			$this->DisplayAsBlock($oPage, $oValue, $aArgs = array(), $sFormPrefix, $oCurrentObj, true /* bDisplayMenu*/);
 		}
 	}
-	
+
+	/**
+	 * @param WebPage $oPage
+	 * @param DBObjectSet $oValue
+	 * @param array $aArgs
+	 * @param $sFormPrefix
+	 * @param $oCurrentObj
+	 * @param $bDisplayMenu
+	 */
 	protected function DisplayAsBlock(WebPage $oPage, DBObjectSet $oValue, $aArgs = array(), $sFormPrefix, $oCurrentObj, $bDisplayMenu)
 	{
 		$oLinksetDef = MetaModel::GetAttributeDef($this->sClass, $this->sAttCode);
@@ -151,7 +166,15 @@ class UILinksWidgetDirect
 			$oBlock->Display($oPage, $this->sInputid, $aParams);
 		}	
 	}
-	
+
+	/**
+	 * @param WebPage $oPage
+	 * @param DBObjectSet $oValue
+	 * @param array $aArgs
+	 * @param $sFormPrefix
+	 * @param $oCurrentObj
+	 * @param array $aButtons
+	 */
 	protected function DisplayEditInPlace(WebPage $oPage, DBObjectSet $oValue, $aArgs = array(), $sFormPrefix, $oCurrentObj, $aButtons = array('create', 'delete'))
 	{
 		$aAttribs = $this->GetTableConfig();

+ 24 - 55
application/ui.linkswidget.class.inc.php

@@ -92,16 +92,18 @@ class UILinksWidget
 			}
 		}
 	}
-	
+
 	/**
 	 * A one-row form for editing a link record
 	 * @param WebPage $oP Web page used for the ouput
-	 * @param DBObject $oLinkedObj The object to which all the elements of the linked set refer to
+	 * @param DBObject $oLinkedObj Remote object
 	 * @param mixed $linkObjOrId Either the object linked or a unique number for new link records to add
-	 * @param Hash $aArgs Extra context arguments
+	 * @param array|Hash $aArgs Extra context arguments
+	 * @param $oCurrentObj The object to which all the elements of the linked set refer to
+	 * @param $iUniqueId A unique identifier of new links
 	 * @return string The HTML fragment of the one-row form
 	 */
-	protected function GetFormRow(WebPage $oP, DBObject $oLinkedObj, $linkObjOrId = null, $aArgs = array(), $oCurrentObj )
+	protected function GetFormRow(WebPage $oP, DBObject $oLinkedObj, $linkObjOrId, $aArgs, $oCurrentObj, $iUniqueId)
 	{
 		$sPrefix = "$this->m_sAttCode{$this->m_sNameSuffix}";
 		$aRow = array();
@@ -115,8 +117,7 @@ class UILinksWidget
 			$aArgs['prefix'] = $sPrefix;
 			$aArgs['wizHelper'] = "oWizardHelper{$this->m_iInputId}{$key}";
 			$aArgs['this'] = $linkObjOrId;
-			$aRow['form::checkbox'] = "<input class=\"selection\" type=\"checkbox\" onClick=\"oWidget".$this->m_iInputId.".OnSelectChange();\" value=\"$key\">";
-			$aRow['form::checkbox'] .= "<input type=\"hidden\" name=\"attr_{$sPrefix}id{$sNameSuffix}\" value=\"$key\">";
+			$aRow['form::checkbox'] = "<input class=\"selection\" data-remote-id=\"$iRemoteObjKey\" data-link-id=\"$key\" data-unique-id=\"$iUniqueId\" type=\"checkbox\" onClick=\"oWidget".$this->m_iInputId.".OnSelectChange();\" value=\"$key\">";
 			foreach($this->m_aEditableFields as $sFieldCode)
 			{
 				$sFieldId = $this->m_iInputId.'_'.$sFieldCode.'['.$linkObjOrId->GetKey().']';
@@ -137,28 +138,25 @@ class UILinksWidget
 				// New link existing only in memory
 				$oNewLinkObj = $linkObjOrId;
 				$iRemoteObjKey = $oNewLinkObj->Get($this->m_sExtKeyToRemote);
-				$oRemoteObj = MetaModel::GetObject($this->m_sRemoteClass, $iRemoteObjKey);
 				$oNewLinkObj->Set($this->m_sExtKeyToMe, $oCurrentObj); // Setting the extkey with the object also fills the related external fields
-				$linkObjOrId = -$iRemoteObjKey;
 			}
 			else
 			{
-				$iRemoteObjKey = -$linkObjOrId;
+				$iRemoteObjKey = $linkObjOrId;
 				$oNewLinkObj = MetaModel::NewObject($this->m_sLinkedClass);
-				$oRemoteObj = MetaModel::GetObject($this->m_sRemoteClass, -$linkObjOrId);
+				$oRemoteObj = MetaModel::GetObject($this->m_sRemoteClass, $iRemoteObjKey);
 				$oNewLinkObj->Set($this->m_sExtKeyToRemote, $oRemoteObj); // Setting the extkey with the object alsoo fills the related external fields
 				$oNewLinkObj->Set($this->m_sExtKeyToMe, $oCurrentObj); // Setting the extkey with the object also fills the related external fields
 			}
-			$sPrefix .= "[$linkObjOrId][";
+			$sPrefix .= "[-$iUniqueId][";
 			$sNameSuffix = "]"; // To make a tabular form
 			$aArgs['prefix'] = $sPrefix;
-			$aArgs['wizHelper'] = "oWizardHelper{$this->m_iInputId}_".(-$linkObjOrId);
+			$aArgs['wizHelper'] = "oWizardHelper{$this->m_iInputId}_".$iUniqueId;
 			$aArgs['this'] = $oNewLinkObj;
-			$aRow['form::checkbox'] = "<input class=\"selection\" type=\"checkbox\" onClick=\"oWidget".$this->m_iInputId.".OnSelectChange();\" value=\"$linkObjOrId\">";
-			$aRow['form::checkbox'] .= "<input type=\"hidden\" name=\"attr_{$sPrefix}id{$sNameSuffix}\" value=\"\">";
+			$aRow['form::checkbox'] = "<input class=\"selection\" data-remote-id=\"$iRemoteObjKey\" data-link-id=\"\" data-unique-id=\"$iUniqueId\" type=\"checkbox\" onClick=\"oWidget".$this->m_iInputId.".OnSelectChange();\" value=\"-$iUniqueId\">";
 			foreach($this->m_aEditableFields as $sFieldCode)
 			{
-				$sFieldId = $this->m_iInputId.'_'.$sFieldCode.'['.$linkObjOrId.']';
+				$sFieldId = $this->m_iInputId.'_'.$sFieldCode.'['.-$iUniqueId.']';
 				$sSafeId = utils::GetSafeId($sFieldId);
 				$sValue = $oNewLinkObj->Get($sFieldCode);
 				$sDisplayValue = $oNewLinkObj->GetEditValue($sFieldCode);
@@ -171,6 +169,7 @@ class UILinksWidget
 			$oP->add_script(
 <<<EOF
 PrepareWidgets();
+oWidget{$this->m_iInputId}.OnLinkAdded($iUniqueId, $iRemoteObjKey);
 EOF
 			);
 		}
@@ -278,21 +277,19 @@ EOF
 		$sHtmlValue .= "<input type=\"hidden\" id=\"{$sFormPrefix}{$this->m_iInputId}\">\n";
 		$oValue->Rewind();
 		$aForm = array();
+		$iAddedId = 1; // Unique id for new links
 		while($oCurrentLink = $oValue->Fetch())
 		{
-			$aRow = array();
 			$oLinkedObj = MetaModel::GetObject($this->m_sRemoteClass, $oCurrentLink->Get($this->m_sExtKeyToRemote));
 			if ($oCurrentLink->IsNew())
 			{
-				$key = -$oLinkedObj->GetKey();
-				$aForm[$key] = $this->GetFormRow($oPage, $oLinkedObj, $oCurrentLink, $aArgs, $oCurrentObj);
+				$key = -($iAddedId++);
 			}
 			else
 			{
 				$key = $oCurrentLink->GetKey();
-				$aForm[$key] = $this->GetFormRow($oPage, $oLinkedObj, $oCurrentLink, $aArgs, $oCurrentObj);
 			}
-
+			$aForm[$key] = $this->GetFormRow($oPage, $oLinkedObj, $oCurrentLink, $aArgs, $oCurrentObj, $key);
 		}
 		$sHtmlValue .= $this->DisplayFormTable($oPage, $this->m_aTableConfig, $aForm);
 		$sDuplicates = ($this->m_bDuplicatesAllowed) ? 'true' : 'false';
@@ -300,7 +297,6 @@ EOF
 		$oPage->add_ready_script(<<<EOF
 		oWidget{$this->m_iInputId} = new LinksWidget('{$this->m_sAttCode}{$this->m_sNameSuffix}', '{$this->m_sClass}', '{$this->m_sAttCode}', '{$this->m_iInputId}', '{$this->m_sNameSuffix}', $sDuplicates, $sWizHelper, '{$this->m_sExtKeyToRemote}');
 		oWidget{$this->m_iInputId}.Init();
-		$('#{$this->m_iInputId}').bind('update_value', function() { $(this).val(oWidget{$this->m_iInputId}.GetUpdatedValue()); })
 EOF
 );
 		$sHtmlValue .= "<span style=\"float:left;\">&nbsp;&nbsp;&nbsp;<img src=\"../images/tv-item-last.gif\">&nbsp;&nbsp;<input id=\"{$this->m_sAttCode}{$this->m_sNameSuffix}_btnRemove\" type=\"button\" value=\"".Dict::S('UI:RemoveLinkedObjectsOf_Class')."\" onClick=\"oWidget{$this->m_iInputId}.RemoveSelected();\" >";
@@ -373,51 +369,25 @@ EOF
 		}
 		if (!$this->m_bDuplicatesAllowed && count($aAlreadyLinkedIds) > 0)
 		{
-			// Positive IDs correspond to existing link records
-			// negative IDs correspond to "remote" objects to be linked
-			$aLinkIds = array();
-			$aRemoteObjIds = array();
-			foreach($aAlreadyLinkedIds as $iId)
-			{
-				if ($iId > 0)
-				{
-					$aLinkIds[] = $iId;
-				}
-				else
-				{
-					$aRemoteObjIds[] = -$iId;
-				}
-			}
-			
-			if (count($aLinkIds) >0)
-			{
-				// Search for the links to find to which "remote" object they are linked
-				$oLinkFilter = new DBObjectSearch($this->m_sLinkedClass);
-				$oLinkFilter->AddCondition('id', $aLinkIds, 'IN');
-				$oLinkSet = new CMDBObjectSet($oLinkFilter);
-				while($oLink = $oLinkSet->Fetch())
-				{
-					$aRemoteObjIds[] = $oLink->Get($this->m_sExtKeyToRemote);
-				}
-			}
-			$oFilter->AddCondition('id', $aRemoteObjIds, 'NOTIN');
+			$oFilter->AddCondition('id', $aAlreadyLinkedIds, 'NOTIN');
 		}
-		$oSet = new CMDBObjectSet($oFilter);
 		$oBlock = new DisplayBlock($oFilter, 'list', false);
 		$oBlock->Display($oP, "ResultsToAdd_{$this->m_sAttCode}", array('menu' => false, 'cssCount'=> '#count_'.$this->m_sAttCode.$this->m_sNameSuffix , 'selection_mode' => true, 'table_id' => 'add_'.$this->m_sAttCode)); // Don't display the 'Actions' menu on the results
 	}
 	
-	public function DoAddObjects(WebPage $oP, $oFullSetFilter, $oCurrentObj)
+	public function DoAddObjects(WebPage $oP, $iMaxAddedId, $oFullSetFilter, $oCurrentObj)
 	{
 		$aLinkedObjectIds = utils::ReadMultipleSelection($oFullSetFilter);
 
+		$iAdditionId = $iMaxAddedId + 1;
 		foreach($aLinkedObjectIds as $iObjectId)
 		{
-			$oLinkedObj = MetaModel::GetObject($this->m_sRemoteClass, $iObjectId);
+			$oLinkedObj = MetaModel::GetObject($this->m_sRemoteClass, $iObjectId, false);
 			if (is_object($oLinkedObj))
 			{
-				$aRow = $this->GetFormRow($oP, $oLinkedObj, -$iObjectId, array(), $oCurrentObj ); // Not yet created link get negative Ids
-				$oP->add($this->DisplayFormRow($oP, $this->m_aTableConfig, $aRow, -$iObjectId)); 
+				$aRow = $this->GetFormRow($oP, $oLinkedObj, $iObjectId, array(), $oCurrentObj, $iAdditionId); // Not yet created link get negative Ids
+				$oP->add($this->DisplayFormRow($oP, $this->m_aTableConfig, $aRow, -$iAdditionId));
+				$iAdditionId++;
 			}
 			else
 			{
@@ -468,4 +438,3 @@ EOF
 		}
 	}
 }
-?>

+ 46 - 26
core/attributedef.class.inc.php

@@ -30,6 +30,7 @@ require_once('ormdocument.class.inc.php');
 require_once('ormstopwatch.class.inc.php');
 require_once('ormpassword.class.inc.php');
 require_once('ormcaselog.class.inc.php');
+require_once('ormlinkset.class.inc.php');
 require_once('htmlsanitizer.class.inc.php');
 require_once(APPROOT.'sources/autoload.php');
 require_once('customfieldshandler.class.inc.php');
@@ -855,7 +856,37 @@ class AttributeLinkedSet extends AttributeDefinition
 	public function GetPrerequisiteAttributes($sClass = null) {return $this->Get("depends_on");}
 	public function GetDefaultValue(DBObject $oHostObject = null)
 	{
-		return DBObjectSet::FromScratch($this->Get('linked_class'));
+		$sLinkClass = $this->GetLinkedClass();
+		$sExtKeyToMe = $this->GetExtKeyToMe();
+
+		// The class to target is not the current class, because if this is a derived class,
+		// it may differ from the target class, then things start to become confusing
+		$oRemoteExtKeyAtt = MetaModel::GetAttributeDef($sLinkClass, $sExtKeyToMe);
+		$sMyClass = $oRemoteExtKeyAtt->GetTargetClass();
+
+		$oMyselfSearch = new DBObjectSearch($sMyClass);
+		if ($oHostObject !== null)
+		{
+			$oMyselfSearch->AddCondition('id', $oHostObject->GetKey(), '=');
+		}
+
+		$oLinkSearch = new DBObjectSearch($sLinkClass);
+		$oLinkSearch->AddCondition_PointingTo($oMyselfSearch, $sExtKeyToMe);
+		if ($this->IsIndirect())
+		{
+			// Join the remote class so that the archive flag will be taken into account
+			$sExtKeyToRemote = $this->GetExtKeyToRemote();
+			$oExtKeyToRemote = MetaModel::GetAttributeDef($sLinkClass, $sExtKeyToRemote);
+			$sRemoteClass = $oExtKeyToRemote->GetTargetClass();
+			if (MetaModel::IsArchivable($sRemoteClass))
+			{
+				$oRemoteSearch = new DBObjectSearch($sRemoteClass);
+				$oLinkSearch->AddCondition_PointingTo($oRemoteSearch, $this->GetExtKeyToRemote());
+			}
+		}
+		$oLinks = new DBObjectSet($oLinkSearch);
+		$oLinkSet = new ormLinkSet($this->GetHostClass(), $this->GetCode(), $oLinks);
+		return $oLinkSet;
 	}
 
 	public function GetTrackingLevel()
@@ -877,7 +908,7 @@ class AttributeLinkedSet extends AttributeDefinition
 
 	public function GetAsHTML($sValue, $oHostObject = null, $bLocalize = true)
 	{
-		if (is_object($sValue) && ($sValue instanceof DBObjectSet))
+		if (is_object($sValue) && ($sValue instanceof ormLinkSet))
 		{
 			$sValue->Rewind();
 			$aItems = array();
@@ -905,7 +936,7 @@ class AttributeLinkedSet extends AttributeDefinition
 
 	public function GetAsXML($sValue, $oHostObject = null, $bLocalize = true)
 	{
-		if (is_object($sValue) && ($sValue instanceof DBObjectSet))
+		if (is_object($sValue) && ($sValue instanceof ormLinkSet))
 		{
 			$sValue->Rewind();
 			$sRes = "<Set>\n";
@@ -954,7 +985,7 @@ class AttributeLinkedSet extends AttributeDefinition
 		$sSepValue = MetaModel::GetConfig()->Get('link_set_value_separator');
 		$sAttributeQualifier = MetaModel::GetConfig()->Get('link_set_attribute_qualifier');
 
-		if (is_object($sValue) && ($sValue instanceof DBObjectSet))
+		if (is_object($sValue) && ($sValue instanceof ormLinkSet))
 		{
 			$sValue->Rewind();
 			$aItems = array();
@@ -1165,7 +1196,7 @@ class AttributeLinkedSet extends AttributeDefinition
 					$oExtKeyFilter->AddCondition($sRemoteAttCode, $sValue, '=');
 					$aReconciliationDesc[] = "$sRemoteAttCode=$sValue";
 				}
-				$oExtKeySet = new CMDBObjectSet($oExtKeyFilter);
+				$oExtKeySet = new DBObjectSet($oExtKeyFilter);
 				switch($oExtKeySet->Count())
 				{
 				case 0:
@@ -1218,7 +1249,7 @@ class AttributeLinkedSet extends AttributeDefinition
 	public function GetForJSON($value)
 	{
 		$aRet = array();
-		if (is_object($value) && ($value instanceof DBObjectSet))
+		if (is_object($value) && ($value instanceof ormLinkSet))
 		{
 			$value->Rewind();
 			while ($oObj = $value->Fetch())
@@ -1311,32 +1342,21 @@ class AttributeLinkedSet extends AttributeDefinition
 		return $oSet;
 	}
 
+	/**
+	 * @param ormLinkSet $val1
+	 * @param ormLinkSet $val2
+	 * @return bool
+	 */
 	public function Equals($val1, $val2)
 	{
-		if ($val1 === $val2) return true;
-
-		if (is_object($val1) != is_object($val2))
+		if ($val1 === $val2)
 		{
-			return false;
+			$bAreEquivalent = true;
 		}
-		if (!is_object($val1))
-		{
-			// string ?
-			// todo = implement this case ?
-			return false;
-		}
-
-		// Note: maintain this algorithm so as to make sure it is strictly equivalent to the one used within DBObject::DBWriteLinks()
-		$sExtKeyToMe = $this->GetExtKeyToMe();
-		$sAdditionalKey = null;
-		if ($this->IsIndirect() && !$this->DuplicatesAllowed())
+		else
 		{
-			$sAdditionalKey = $this->GetExtKeyToRemote();
+			$bAreEquivalent = $val1->Equals($val2);
 		}
-		$oComparator = new DBObjectSetComparator($val1, $val2, array($sExtKeyToMe), $sAdditionalKey);
-		$aChanges = $oComparator->GetDifferences();
-
-		$bAreEquivalent = (count($aChanges['added']) == 0) && (count($aChanges['removed']) == 0) && (count($aChanges['modified']) == 0);
 		return $bAreEquivalent;
 	}
 

+ 13 - 84
core/dbobject.class.php

@@ -212,35 +212,7 @@ abstract class DBObject implements iDisplay
 		{
 			if (!$oAttDef->IsLinkSet()) continue;
 
-			// Load the link information
-			$sLinkClass = $oAttDef->GetLinkedClass();
-			$sExtKeyToMe = $oAttDef->GetExtKeyToMe();
-
-			// The class to target is not the current class, because if this is a derived class,
-			// it may differ from the target class, then things start to become confusing
-			$oRemoteExtKeyAtt = MetaModel::GetAttributeDef($sLinkClass, $sExtKeyToMe);
-			$sMyClass = $oRemoteExtKeyAtt->GetTargetClass();
-
-			$oMyselfSearch = new DBObjectSearch($sMyClass);
-			$oMyselfSearch->AddCondition('id', $this->m_iKey, '=');
-
-			$oLinkSearch = new DBObjectSearch($sLinkClass);
-			$oLinkSearch->AddCondition_PointingTo($oMyselfSearch, $sExtKeyToMe);
-			if ($oAttDef->IsIndirect())
-			{
-				// Join the remote class so that the archive flag will be taken into account
-				$sExtKeyToRemote = $oAttDef->GetExtKeyToRemote();
-				$oExtKeyToRemote = MetaModel::GetAttributeDef($sLinkClass, $sExtKeyToRemote);
-				$sRemoteClass = $oExtKeyToRemote->GetTargetClass();
-				if (MetaModel::IsArchivable($sRemoteClass))
-				{
-					$oRemoteSearch = new DBObjectSearch($sRemoteClass);
-					$oLinkSearch->AddCondition_PointingTo($oRemoteSearch, $oAttDef->GetExtKeyToRemote());
-				}
-			}
-			$oLinks = new DBObjectSet($oLinkSearch);
-
-			$this->m_aCurrValues[$sAttCode] = $oLinks;
+			$this->m_aCurrValues[$sAttCode] = $oAttDef->GetDefaultValue($this);
 			$this->m_aOrigValues[$sAttCode] = clone $this->m_aCurrValues[$sAttCode];
 			$this->m_aLoadedAtt[$sAttCode] = true;
 		}
@@ -431,33 +403,15 @@ abstract class DBObject implements iDisplay
 				}
 			}
 		}
-		if($oAttDef->IsLinkSet())
+		if ($oAttDef->IsLinkSet() && ($value != null))
 		{
-			if (is_null($value))
-			{
-				// Normalize
-				$value = DBObjectSet::FromScratch($oAttDef->GetLinkedClass());
-			}
-			else
-			{
-				if ((get_class($value) != 'DBObjectSet') && !is_subclass_of($value, 'DBObjectSet'))
-				{
-					throw new CoreUnexpectedValue("expecting a set of persistent objects (found a '".get_class($value)."'), setting default value (empty list)");
-				}
-			}
-
-			$oObjectSet = $value;
-			$sSetClass = $oObjectSet->GetClass();
-			$sLinkClass = $oAttDef->GetLinkedClass();
-			// not working fine :-(   if (!is_subclass_of($sSetClass, $sLinkClass))
-			if ($sSetClass != $sLinkClass)
-			{
-				throw new CoreUnexpectedValue("expecting a set of '$sLinkClass' objects (found a set of '$sSetClass'), setting default value (empty list)");
-			}
+			$realvalue = clone $this->m_aCurrValues[$sAttCode];
+			$realvalue->UpdateFromCompleteList($value);
+		}
+		else
+		{
+			$realvalue = $oAttDef->MakeRealValue($value, $this);
 		}
-
-		$realvalue = $oAttDef->MakeRealValue($value, $this);
-
 		$this->_Set($sAttCode, $realvalue);
 
 		foreach (MetaModel::ListMetaAttributes(get_class($this), $sAttCode) as $sMetaAttCode => $oMetaAttDef)
@@ -606,7 +560,7 @@ abstract class DBObject implements iDisplay
 			$value = $this->m_aCurrValues[$sAttCode];
 		}
 
-		if ($value instanceof DBObjectSet)
+		if ($value instanceof ormLinkSet)
 		{
 			$value->Rewind();
 		}
@@ -1561,39 +1515,14 @@ abstract class DBObject implements iDisplay
 	// used both by insert/update
 	private function DBWriteLinks()
 	{
-		foreach(MetaModel::ListAttributeDefs(get_class($this)) as $sAttCode=>$oAttDef)
+		foreach(MetaModel::ListAttributeDefs(get_class($this)) as $sAttCode => $oAttDef)
 		{
 			if (!$oAttDef->IsLinkSet()) continue;
 			if (!array_key_exists($sAttCode, $this->m_aTouchedAtt)) continue;
 			if (array_key_exists($sAttCode, $this->m_aModifiedAtt) && ($this->m_aModifiedAtt[$sAttCode] == false)) continue;
-				
-			// Note: any change to this algorithm must be reproduced into the implementation of AttributeLinkSet::Equals()
-			$sExtKeyToMe = $oAttDef->GetExtKeyToMe();
-			$sAdditionalKey = null;
-			if ($oAttDef->IsIndirect() && !$oAttDef->DuplicatesAllowed())
-			{
-				$sAdditionalKey = $oAttDef->GetExtKeyToRemote();
-			}
-			$oComparator = new DBObjectSetComparator($this->m_aOrigValues[$sAttCode], $this->Get($sAttCode), array($sExtKeyToMe), $sAdditionalKey);
-			$aChanges = $oComparator->GetDifferences();
-			
-			foreach($aChanges['added'] as $oLink)
-			{
-				// Make sure that the objects in the set point to "this"
-				$oLink->Set($oAttDef->GetExtKeyToMe(), $this->m_iKey);
-				$id = $oLink->DBWrite();
-			}
-			
-			foreach($aChanges['modified'] as $oLink)
-			{
-				// Objects in the set either remain attached or have been detached -> leave the link as is
-				$oLink->DBWrite();
-			}
-			
-			foreach($aChanges['removed'] as $oLink)
-			{
-				$oLink->DBDelete();
-			}
+
+			$oLinkSet = $this->m_aCurrValues[$sAttCode];
+			$oLinkSet->DBWrite($this);
 		}
 	}
 

+ 63 - 0
core/dbobjectiterator.php

@@ -0,0 +1,63 @@
+<?php
+// Copyright (C) 2010-2017 Combodo SARL
+//
+//   This file is part of iTop.
+//
+//   iTop is free software; you can redistribute it and/or modify	
+//   it under the terms of the GNU Affero General Public License as published by
+//   the Free Software Foundation, either version 3 of the License, or
+//   (at your option) any later version.
+//
+//   iTop is distributed in the hope that it will be useful,
+//   but WITHOUT ANY WARRANTY; without even the implied warranty of
+//   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+//   GNU Affero General Public License for more details.
+//
+//   You should have received a copy of the GNU Affero General Public License
+//   along with iTop. If not, see <http://www.gnu.org/licenses/>
+
+
+/**
+ * A set of persistent objects, could be heterogeneous as long as the objects in the set have a common ancestor class 
+ *
+ * @package     iTopORM
+ * @copyright   Copyright (C) 2010-2017 Combodo SARL
+ * @license     http://opensource.org/licenses/AGPL-3.0
+ */
+interface iDBObjectSetIterator extends Countable
+{
+	/**
+	 * The class of the objects of the collection (at least a common ancestor)
+	 *
+	 * @return string
+	 */
+	public function GetClass();
+
+	/**
+	 * The total number of objects in the collection
+	 *
+	 * @return int
+	 */
+	public function Count();
+
+	/**
+	 * Reset the cursor to the first item in the collection. Equivalent to Seek(0)
+	 *
+	 * @return DBObject The fetched object or null when at the end
+	 */
+	public function Rewind();
+
+	/**
+	 * Position the cursor to the given 0-based position
+	 *
+	 * @param int $iRow
+	 */
+	public function Seek($iPosition);
+
+	/**
+	 * Fetch the object at the current position in the collection and move the cursor to the next position.
+	 *
+	 * @return DBObject The fetched object or null when at the end
+	 */
+	public function Fetch();
+}

+ 0 - 1
core/dbobjectsearch.class.php

@@ -387,7 +387,6 @@ class DBObjectSearch extends DBSearch
 				return;
 			}
 		}
-		MyHelpers::CheckKeyInArray('operator', $sOpCode, $oFilterDef->GetOperators());
 		// Parse search strings if needed and if the filter code corresponds to a valid attcode
 		if($bParseSeachString && MetaModel::IsValidAttCode($this->GetClass(), $sFilterCode))
 		{

+ 14 - 27
core/dbobjectset.class.php

@@ -16,6 +16,7 @@
 //   You should have received a copy of the GNU Affero General Public License
 //   along with iTop. If not, see <http://www.gnu.org/licenses/>
 
+require_once('dbobjectiterator.php');
 
 /**
  * Object set management
@@ -30,7 +31,7 @@
  *
  * @package     iTopORM
  */
-class DBObjectSet
+class DBObjectSet implements iDBObjectSetIterator
 {
 	/**
 	 * @var array
@@ -236,7 +237,7 @@ class DBObjectSet
 	 * 
 	 * @param string $sClass The class (or an ancestor) for the objects to be added in this set
 	 * 
-	 * @return DBObject The empty set
+	 * @return DBObjectSet The empty set
 	 */
 	static public function FromScratch($sClass)
 	{
@@ -922,22 +923,6 @@ class DBObjectSet
 		return $oComparator->SetsAreEquivalent();
 	}
 
-	protected function GetObjectAt($iIndex)
-	{
-		if (!$this->m_bLoaded) $this->Load();
-		
-		// Save the current position for iteration
-		$iCurrPos = $this->m_iCurrRow;
-		
-		$this->Seek($iIndex);
-		$oObject = $this->Fetch();
-		
-		// Restore the current position for iteration
-		$this->Seek($this->m_iCurrRow);
-		
-		return $oObject;
-	}
-	
 	/**
 	 * Build a new set (in memory) made of objects of the given set which are NOT present in the current set
 	 * 
@@ -1199,19 +1184,27 @@ class DBObjectSetComparator
 	protected $aIDs1;
 	protected $aIDs2;
 	protected $aExcludedColumns;
+
+	/**
+	 * @var iDBObjectSetIterator
+	 */
 	protected $oSet1;
+	/**
+	 * @var iDBObjectSetIterator
+	 */
 	protected $oSet2;
+
 	protected $sAdditionalKeyColumn;
 	protected $aAdditionalKeys;
 	
 	/**
 	 * Initializes the comparator
-	 * @param DBObjectSet $oSet1 The first set of objects to compare, or null
-	 * @param DBObjectSet $oSet2 The second set of objects to compare, or null
+	 * @param iDBObjectSetIterator $oSet1 The first set of objects to compare, or null
+	 * @param iDBObjectSetIterator $oSet2 The second set of objects to compare, or null
 	 * @param array $aExcludedColumns The list of columns (= attribute codes) to exclude from the comparison
 	 * @param string $sAdditionalKeyColumn The attribute code of an additional column to be considered as a key indentifying the object (useful for n:n links)
 	 */
-	public function __construct($oSet1, $oSet2, $aExcludedColumns = array(), $sAdditionalKeyColumn = null)
+	public function __construct(iDBObjectSetIterator $oSet1, iDBObjectSetIterator $oSet2, $aExcludedColumns = array(), $sAdditionalKeyColumn = null)
 	{
 		$this->aFingerprints1 = null;
 		$this->aFingerprints2 = null;
@@ -1237,9 +1230,6 @@ class DBObjectSetComparator
 			
 			if ($this->oSet1 !== null)
 			{
-				$aAliases = $this->oSet1->GetSelectedClasses();
-				if (count($aAliases) > 1) throw new Exception('DBObjectSetComparator does not support Sets with more than one column. $oSet1: ('.print_r($aAliases, true).')');
-				
 				$this->oSet1->Rewind();
 				while($oObj = $this->oSet1->Fetch())
 				{
@@ -1255,9 +1245,6 @@ class DBObjectSetComparator
 				
 			if ($this->oSet2 !== null)
 			{
-				$aAliases = $this->oSet2->GetSelectedClasses();
-				if (count($aAliases) > 1) throw new Exception('DBObjectSetComparator does not support Sets with more than one column. $oSet2: ('.print_r($aAliases, true).')');
-				
 				$this->oSet2->Rewind();
 				while($oObj = $this->oSet2->Fetch())
 				{

+ 19 - 2
core/metamodel.class.php

@@ -631,6 +631,10 @@ abstract class MetaModel
 	private static $m_aIgnoredAttributes = array(); //array of ("classname" => array of ("attcode"))
 	private static $m_aEnumToMeta = array(); // array of  ("classname" => array of ("attcode" => array of ("metaattcode" => oMetaAttDef))
 
+	/**
+	 * @param $sClass
+	 * @return AttributeDefinition[]
+	 */
 	final static public function ListAttributeDefs($sClass)
 	{
 		self::_check_subclass($sClass);	
@@ -4855,10 +4859,23 @@ abstract class MetaModel
 		return $oObj->GetHyperLink();
 	}
 
-	public static function NewObject($sClass)
+	/**
+	 * @param string $sClass
+	 * @param array|null $aValues array of attcode => value
+	 * @return DBObject
+	 */
+	public static function NewObject($sClass, $aValues = null)
 	{
 		self::_check_subclass($sClass);
-		return new $sClass();
+		$oRet = new $sClass();
+		if (is_array($aValues))
+		{
+			foreach ($aValues as $sAttCode => $value)
+			{
+				$oRet->Set($sAttCode, $value);
+			}
+		}
+		return $oRet;
 	}	
 
 	public static function GetNextKey($sClass)

+ 563 - 0
core/ormlinkset.class.inc.php

@@ -0,0 +1,563 @@
+<?php
+// Copyright (C) 2010-2017 Combodo SARL
+//
+//   This file is part of iTop.
+//
+//   iTop is free software; you can redistribute it and/or modify	
+//   it under the terms of the GNU Affero General Public License as published by
+//   the Free Software Foundation, either version 3 of the License, or
+//   (at your option) any later version.
+//
+//   iTop is distributed in the hope that it will be useful,
+//   but WITHOUT ANY WARRANTY; without even the implied warranty of
+//   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+//   GNU Affero General Public License for more details.
+//
+//   You should have received a copy of the GNU Affero General Public License
+//   along with iTop. If not, see <http://www.gnu.org/licenses/>
+
+require_once('dbobjectiterator.php');
+
+
+/**
+ * The value for an attribute representing a set of links between the host object and "remote" objects
+ *
+ * @package     iTopORM
+ * @copyright   Copyright (C) 2010-2017 Combodo SARL
+ * @license     http://opensource.org/licenses/AGPL-3.0
+ */
+
+class ormLinkSet implements iDBObjectSetIterator, Iterator, SeekableIterator
+{
+	protected $sHostClass; // subclass of DBObject
+	protected $sAttCode; // xxxxxx_list
+	protected $sClass; // class of the links
+
+	/**
+	 * @var DBObjectSet
+	 */
+	protected $oOriginalSet;
+
+	/**
+	 * @var DBObject[] array of iObjectId => DBObject
+	 */
+	protected $aOriginalObjects = null;
+
+	/**
+	 * @var bool
+	 */
+	protected $bHasDelta = false;
+
+	/**
+	 * Object from the original set, minus the removed objects
+	 * @var DBObject[] array of iObjectId => DBObject
+	 */
+	protected $aPreserved;
+
+	/**
+	 * @var DBObject[] New items
+	 */
+	protected $aAdded = array();
+
+	/**
+	 * @var DBObject[] Modified items (could also be found in aPreserved)
+	 */
+	protected $aModified = array();
+
+	/**
+	 * @var int[] Removed items
+	 */
+	protected $aRemoved = array();
+
+	/**
+	 * @var int Position in the collection
+	 */
+	protected $iCursor = 0;
+
+	/**
+	 * ormLinkSet constructor.
+	 * @param $sHostClass
+	 * @param $sAttCode
+	 * @param DBObjectSet|null $oOriginalSet
+	 * @throws Exception
+	 */
+	public function __construct($sHostClass, $sAttCode, DBObjectSet $oOriginalSet = null)
+	{
+		$this->sHostClass = $sHostClass;
+		$this->sAttCode = $sAttCode;
+		$this->oOriginalSet = $oOriginalSet ? clone $oOriginalSet : null;
+
+		$oAttDef = MetaModel::GetAttributeDef($sHostClass, $sAttCode);
+		if (!$oAttDef instanceof AttributeLinkedSet)
+		{
+			throw new Exception("ormLinkSet: $sAttCode is not a link set");
+		}
+		$this->sClass = $oAttDef->GetLinkedClass();
+		if ($oOriginalSet && ($oOriginalSet->GetClass() != $this->sClass))
+		{
+			throw new Exception("ormLinkSet: wrong class for the original set, found {$oOriginalSet->GetClass()} while expecting {$oAttDef->GetLinkedClass()}");
+		}
+	}
+
+	public function GetFilter()
+	{
+		return clone $this->oOriginalSet->GetFilter();
+	}
+
+	/**
+	 * Specify the subset of attributes to load (for each class of objects) before performing the SQL query for retrieving the rows from the DB
+	 *
+	 * @param hash $aAttToLoad Format: alias => array of attribute_codes
+	 *
+	 * @return void
+	 */
+	public function OptimizeColumnLoad($aAttToLoad)
+	{
+		$this->oOriginalSet->OptimizeColumnLoad($aAttToLoad);
+	}
+
+	/**
+	 * @param DBObject $oLink
+	 */
+	public function AddItem(DBObject $oLink)
+	{
+		assert($oLink instanceof $this->sClass);
+		// No impact on the iteration algorithm
+		$this->aAdded[] = $oLink;
+		$this->bHasDelta = true;
+	}
+
+	/**
+	 * @param $iObjectId
+	 */
+	public function RemoveItem($iObjectId)
+	{
+		if (array_key_exists($iObjectId, $this->aPreserved))
+		{
+			unset($this->aPreserved[$iObjectId]);
+			$this->aRemoved[$iObjectId] = $iObjectId;
+			$this->bHasDelta = true;
+		}
+	}
+
+	/**
+	 * @param DBObject $oLink
+	 */
+	public function ModifyItem(DBObject $oLink)
+	{
+		assert($oLink instanceof $this->sClass);
+		$iObjectId = $oLink->GetKey();
+		$this->aModified[$iObjectId] = $oLink;
+		$this->bHasDelta = true;
+	}
+
+	protected function LoadOriginalIds()
+	{
+		if ($this->aOriginalObjects === null)
+		{
+			if ($this->oOriginalSet)
+			{
+				$this->aOriginalObjects = $this->oOriginalSet->ToArray();
+				$this->aPreserved = $this->aOriginalObjects; // Copy (not effective until aPreserved gets modified)
+				foreach ($this->aRemoved as $iObjectId)
+				{
+					if (array_key_exists($iObjectId, $this->aPreserved))
+					{
+						unset($this->aPreserved[$iObjectId]);
+					}
+				}
+			}
+			else
+			{
+
+				// Nothing to load
+				$this->aOriginalObjects = [];
+				$this->aPreserved = [];
+			}
+		}
+	}
+
+	/**
+	 * The class of the objects of the collection (at least a common ancestor)
+	 *
+	 * @return string
+	 */
+	public function GetClass()
+	{
+		return $this->sClass;
+	}
+
+	/**
+	 * The total number of objects in the collection
+	 *
+	 * @return int
+	 */
+	public function Count()
+	{
+		$this->LoadOriginalIds();
+		$iRet = count($this->aPreserved) + count($this->aAdded);
+		return $iRet;
+	}
+
+	/**
+	 * Position the cursor to the given 0-based position
+	 *
+	 * @param $iPosition
+	 * @throws Exception
+	 * @internal param int $iRow
+	 */
+	public function Seek($iPosition)
+	{
+		$this->LoadOriginalIds();
+
+		$iCount = $this->Count();
+		if ($iPosition >= $iCount)
+		{
+			throw new Exception("Invalid position $iPosition: the link set is made of $iCount items.");
+		}
+		$this->rewind();
+		for($iPos = 0 ; $iPos < $iPosition ; $iPos++)
+		{
+			$this->next();
+		}
+	}
+
+	/**
+	 * Fetch the object at the current position in the collection and move the cursor to the next position.
+	 *
+	 * @return DBObject|null The fetched object or null when at the end
+	 */
+	public function Fetch()
+	{
+		$this->LoadOriginalIds();
+
+		$ret = $this->current();
+		if ($ret === false)
+		{
+			$ret = null;
+		}
+		$this->next();
+		return $ret;
+	}
+
+	/**
+	 * Return the current element
+	 * @link http://php.net/manual/en/iterator.current.php
+	 * @return mixed Can return any type.
+	 */
+	public function current()
+	{
+		$this->LoadOriginalIds();
+
+		$iPreservedCount = count($this->aPreserved);
+		if ($this->iCursor < $iPreservedCount)
+		{
+			$oRet = current($this->aPreserved);
+		}
+		else
+		{
+			$oRet = current($this->aAdded);
+		}
+		return $oRet;
+	}
+
+	/**
+	 * Move forward to next element
+	 * @link http://php.net/manual/en/iterator.next.php
+	 * @return void Any returned value is ignored.
+	 */
+	public function next()
+	{
+		$this->LoadOriginalIds();
+
+		$iPreservedCount = count($this->aPreserved);
+		if ($this->iCursor < $iPreservedCount)
+		{
+			next($this->aPreserved);
+		}
+		else
+		{
+			next($this->aAdded);
+		}
+		// Increment AFTER moving the internal cursors because when starting aAdded, we must leave it intact
+		$this->iCursor++;
+	}
+
+	/**
+	 * Return the key of the current element
+	 * @link http://php.net/manual/en/iterator.key.php
+	 * @return mixed scalar on success, or null on failure.
+	 */
+	public function key()
+	{
+		return $this->iCursor;
+	}
+
+	/**
+	 * Checks if current position is valid
+	 * @link http://php.net/manual/en/iterator.valid.php
+	 * @return boolean The return value will be casted to boolean and then evaluated.
+	 * Returns true on success or false on failure.
+	 */
+	public function valid()
+	{
+		$this->LoadOriginalIds();
+
+		$iCount = $this->Count();
+		$bRet = ($this->iCursor < $iCount);
+		return $bRet;
+	}
+
+	/**
+	 * Rewind the Iterator to the first element
+	 * @link http://php.net/manual/en/iterator.rewind.php
+	 * @return void Any returned value is ignored.
+	 */
+	public function rewind()
+	{
+		$this->LoadOriginalIds();
+
+		$this->iCursor = 0;
+		reset($this->aPreserved);
+		reset($this->aAdded);
+	}
+
+	public function HasDelta()
+	{
+		return $this->bHasDelta;
+	}
+
+	/**
+	 * This method has been designed specifically for AttributeLinkedSet:Equals and as such it assumes that the passed argument is a clone of this.
+	 * @param ormLinkSet $oFellow
+	 * @return bool|null
+	 * @throws Exception
+	 */
+	public function Equals(ormLinkSet $oFellow)
+	{
+		$bRet = null;
+		if ($this === $oFellow)
+		{
+			$bRet = true;
+		}
+		else
+		{
+			if ( ($this->oOriginalSet !== $oFellow->oOriginalSet)
+			&& ($this->oOriginalSet->GetFilter()->ToOQL() != $oFellow->oOriginalSet->GetFilter()->ToOQL()) )
+			{
+				throw new Exception('ormLinkSet::Equals assumes that compared link sets have the same original scope');
+			}
+			if ($this->HasDelta())
+			{
+				throw new Exception('ormLinkSet::Equals assumes that left link set had no delta');
+			}
+			$bRet = !$oFellow->HasDelta();
+		}
+		return $bRet;
+	}
+
+	public function UpdateFromCompleteList(iDBObjectSetIterator $oFellow)
+	{
+		if ($oFellow === $this)
+		{
+			throw new Exception('ormLinkSet::UpdateFromCompleteList assumes that the passed link set is at least a clone of the current one');
+		}
+		$bUpdateFromDelta = false;
+		if ($oFellow instanceof ormLinkSet)
+		{
+			if ( ($this->oOriginalSet === $oFellow->oOriginalSet)
+				|| ($this->oOriginalSet->GetFilter()->ToOQL() == $oFellow->oOriginalSet->GetFilter()->ToOQL()) )
+			{
+				$bUpdateFromDelta = true;
+			}
+		}
+
+		if ($bUpdateFromDelta)
+		{
+			// Same original set -> simply update the delta
+			$this->iCursor = 0;
+			$this->aAdded = $oFellow->aAdded;
+			$this->aRemoved = $oFellow->aRemoved;
+			$this->aModified = $oFellow->aModified;
+			$this->aPreserved = $oFellow->aPreserved;
+			$this->bHasDelta = $oFellow->bHasDelta;
+		}
+		else
+		{
+			// For backward compatibility reasons, let's rebuild a delta...
+
+			// Reset the delta
+			$this->iCursor = 0;
+			$this->aAdded = array();
+			$this->aRemoved = array();
+			$this->aModified = array();
+			$this->aPreserved = $this->aOriginalObjects;
+			$this->bHasDelta = false;
+
+			/** @var AttributeLinkedSet $oAttDef */
+			$oAttDef = MetaModel::GetAttributeDef($this->sHostClass, $this->sAttCode);
+			$sExtKeyToMe = $oAttDef->GetExtKeyToMe();
+			$sAdditionalKey = null;
+			if ($oAttDef->IsIndirect() && !$oAttDef->DuplicatesAllowed())
+			{
+				$sAdditionalKey = $oAttDef->GetExtKeyToRemote();
+			}
+			// Compare both collections by iterating the whole sets, order them, a build a fingerprint based on meaningful data (what make the difference)
+			$oComparator = new DBObjectSetComparator($this, $oFellow, array($sExtKeyToMe), $sAdditionalKey);
+			$aChanges = $oComparator->GetDifferences();
+			foreach ($aChanges['added'] as $oLink)
+			{
+				$this->AddItem($oLink);
+			}
+
+			foreach ($aChanges['modified'] as $oLink)
+			{
+				$this->ModifyItem($oLink);
+			}
+
+			foreach ($aChanges['removed'] as $oLink)
+			{
+				$this->RemoveItem($oLink->GetKey());
+			}
+		}
+	}
+
+	/**
+	 * @param DBObject $oHostObject
+	 */
+	public function DBWrite(DBObject $oHostObject)
+	{
+		/** @var AttributeLinkedSet $oAttDef */
+		$oAttDef = MetaModel::GetAttributeDef(get_class($oHostObject), $this->sAttCode);
+		$sExtKeyToMe = $oAttDef->GetExtKeyToMe();
+		$sExtKeyToRemote = $oAttDef->IsIndirect() ? $oAttDef->GetExtKeyToRemote() : 'n/a';
+
+		$aCheckLinks = array();
+		$aCheckRemote = array();
+		foreach ($this->aAdded as $oLink)
+		{
+			if ($oLink->IsNew())
+			{
+				if ($oAttDef->IsIndirect() && !$oAttDef->DuplicatesAllowed())
+				{
+					//todo: faire un test qui passe dans cette branche !
+					$aCheckRemote[] = $oLink->Get($sExtKeyToRemote);
+				}
+			}
+			else
+			{
+				//todo: faire un test qui passe dans cette branche !
+				$aCheckLinks[] = $oLink->GetKey();
+			}
+		}
+		foreach ($this->aRemoved as $iLinkId)
+		{
+			$aCheckLinks[] = $iLinkId;
+		}
+		foreach ($this->aModified as $iLinkId => $oLink)
+		{
+			$aCheckLinks[] = $oLink->GetKey();
+		}
+
+		// Critical section : serialize any write access to these links
+		//
+		$oMtx = new iTopMutex('Write-'.$this->sClass);
+		$oMtx->Lock();
+
+		// Check for the existing links
+		//
+		if (count($aCheckLinks) > 0)
+		{
+			$oSearch = new DBObjectSearch($this->sClass);
+			$oSearch->AddCondition('id', $aCheckLinks, 'IN');
+			$oSet = new DBObjectSet($oSearch);
+			/** @var DBObject[] $aExistingLinks */
+			$aExistingLinks = $oSet->ToArray();
+		}
+
+		// Check for the existing remote objects
+		//
+		if (count($aCheckRemote) > 0)
+		{
+			$oSearch = new DBObjectSearch($this->sClass);
+			$oSearch->AddCondition($sExtKeyToMe, $oHostObject->GetKey(), '=');
+			$oSearch->AddCondition($sExtKeyToRemote, $aCheckRemote, 'IN');
+			$oSet = new DBObjectSet($oSearch);
+			/** @var Int[] $aExistingRemote */
+			$aExistingRemote = $oSet->GetColumnAsArray($sExtKeyToRemote);
+		}
+
+		// Write the links according to the existing links
+		//
+		foreach ($this->aAdded as $oLink)
+		{
+			// Make sure that the objects in the set point to "this"
+			$oLink->Set($sExtKeyToMe, $oHostObject->GetKey());
+
+			if ($oLink->IsNew())
+			{
+				if (count($aCheckRemote) > 0)
+				{
+					if (in_array($oLink->Get($sExtKeyToRemote), $aExistingRemote))
+					{
+						// Do not create a duplicate
+						continue;
+					}
+				}
+
+			}
+			else
+			{
+				if (!array_key_exists($oLink->GetKey(), $aExistingLinks))
+				{
+					$oLink->DBClone();
+				}
+			}
+			$oLink->DBWrite();
+		}
+		foreach ($this->aRemoved as $iLinkId)
+		{
+			if (array_key_exists($iLinkId, $aExistingLinks))
+			{
+				$oLink = $aExistingLinks[$iLinkId];
+				if ($oAttDef->IsIndirect())
+				{
+					$oLink->DBDelete();
+				}
+				else
+				{
+					$oExtKeyToRemote = MetaModel::GetAttributeDef($this->sClass, $sExtKeyToMe);
+					if ($oExtKeyToRemote->IsNullAllowed())
+					{
+						if ($oLink->Get($sExtKeyToMe) == $oHostObject->GetKey())
+						{
+							// Detach the link object from this
+							$oLink->Set($sExtKeyToMe, 0);
+							$oLink->DBUpdate();
+						}
+					}
+					else
+					{
+						$oLink->DBDelete();
+					}
+				}
+			}
+		}
+		// Note: process modifications at the end: if a link to remove has also been listed as modified, then it will be gracefully ignored
+		foreach ($this->aModified as $iLinkId => $oLink)
+		{
+			if (array_key_exists($oLink->GetKey(), $aExistingLinks))
+			{
+				$oLink->DBUpdate();
+			}
+			else
+			{
+				$oLink->DBClone();
+			}
+		}
+
+		// End of the critical section
+		//
+		$oMtx->Unlock();
+	}
+}

+ 2 - 6
datamodels/2.x/itop-tickets/main.itop-tickets.php

@@ -195,8 +195,6 @@ class _Ticket extends cmdbAbstractObject
 			}
 		}
 		
-		$oContactsSet = DBObjectSet::FromScratch('lnkContactToTicket');
-		
 		$sContextKey = 'itop-tickets/relation_context/'.get_class($this).'/impacts/down';
 		$aContextDefs = DisplayableGraph::GetContextDefinitions($sContextKey, true, array('this' => $this));
 		$aDefaultContexts = array();
@@ -219,7 +217,6 @@ class _Ticket extends cmdbAbstractObject
 			{
 				$oObj = $oNode->GetProperty('object');
 				$iKey = $oObj->GetKey();
-				$sRootClass = MetaModel::GetRootClass(get_class($oObj));
 				$aGraphObjects[get_class($oObj).'::'.$iKey] = $oNode->GetProperty('object');
 			}
 		}
@@ -235,7 +232,6 @@ class _Ticket extends cmdbAbstractObject
 				{
 					$oObj = $oNode->GetProperty('object');
 					$iKey = $oObj->GetKey();
-					$sRootClass = MetaModel::GetRootClass(get_class($oObj));
 					$aGraphObjects[get_class($oObj).'::'.$iKey] = $oNode->GetProperty('object');
 				}
 			}
@@ -247,8 +243,8 @@ class _Ticket extends cmdbAbstractObject
 			$sRootClass = MetaModel::GetRootClass(get_class($oObj));
 			switch ($sRootClass)
 			{
-				case 'FunctionalCI':		
-				// Only link FunctionalCIs which are not already linked to the ticket
+				case 'FunctionalCI':
+				// Only FunctionalCIs which are not already linked to the ticket
 				if (!array_key_exists($iKey, $aCIsToImpactCode) || ($aCIsToImpactCode[$iKey] != 'not_impacted'))
 				{
 					$oNewLink = new lnkFunctionalCIToTicket();

+ 109 - 26
js/linkswidget.js

@@ -9,39 +9,60 @@ function LinksWidget(id, sClass, sAttCode, iInputId, sSuffix, bDuplicates, oWizH
 	this.bDuplicates = bDuplicates;
 	this.oWizardHelper = oWizHelper;
 	this.sExtKeyToRemote = sExtKeyToRemote;
+    this.iMaxAddedId = 0;
+	this.aAdded = [];
+	this.aRemoved = [];
+	this.aModified = {};
 	var me = this;
+
 	this.Init = function()
 	{
 		// make sure that the form is clean
 		$('#linkedset_'+this.id+' .selection').each( function() { this.checked = false; });
 		$('#'+this.id+'_btnRemove').attr('disabled','disabled');
-		$('#'+this.id+'_linksToRemove').val('');
-		
+
 		$('#linkedset_'+me.id).on('remove', function() {
 			// prevent having the dlg div twice
 			$('#dlg_'+me.id).remove();
 		});
+
+        $('#linkedset_'+me.id+' :input').off('change').on('change', function() {
+			if (!($(this).hasClass('selection'))) {
+				var oCheckbox = $(this).closest('tr').find('.selection');
+				var iLink = oCheckbox.attr('data-link-id');
+				var iUniqueId = oCheckbox.attr('data-unique-id');
+				var sAttCode = $(this).closest('.attribute-edit').attr('data-attcode');
+				var value = $(this).val();
+				return me.OnValueChange(iLink, iUniqueId, sAttCode, value);
+			}
+			return true;
+        });
+
+		var oInput = $('#'+this.iInputId);
+		oInput.bind('update_value', function() { $(this).val(me.GetUpdatedValue()); });
+		oInput.closest('form').submit(function() { return me.OnFormSubmit(); });
 	};
 	
 	this.RemoveSelected = function()
 	{
 		var my_id = '#'+me.id;
-		$('#linkedset_'+me.id+' .selection:checked').each(
-			function()
+		$('#linkedset_'+me.id+' .selection:checked').each(function() {
+			$(my_id+'_row_'+this.value).remove();
+			var iLink = $(this).attr('data-link-id');
+			if (iLink > 0)
 			{
-				$linksToRemove = $(my_id+'_linksToRemove');
-				prevValue = $linksToRemove.val();
-				if (prevValue != '')
-				{
-					$linksToRemove.val(prevValue + ',' + this.value);
-				}
-				else
+				me.aRemoved.push(iLink);
+				if (me.aModified.hasOwnProperty(iLink))
 				{
-					$linksToRemove.val(this.value);
+					delete me.aModified[iLink];
 				}
-				$(my_id+'_row_'+this.value).remove();
 			}
-		);
+			else
+			{
+				var iUniqueId = $(this).attr('data-unique-id');
+				me.aAdded[iUniqueId] = null;
+			}
+		});
 		// Disable the button since all the selected items have been removed
 		$(my_id+'_btnRemove').attr('disabled','disabled');
 		// Re-run the zebra plugin to properly highlight the remaining lines & and take into account the removed ones
@@ -115,13 +136,11 @@ function LinksWidget(id, sClass, sAttCode, iInputId, sSuffix, bDuplicates, oWizH
 		});
 		
 		// Gather the already linked target objects
-		theMap.aAlreadyLinked = new Array();
-		$('#linkedset_'+me.id+' .selection:input').each(
-			function(i)
-			{
-				theMap.aAlreadyLinked.push(this.value);
-			}
-		);
+		theMap.aAlreadyLinked = [];
+		$('#linkedset_'+me.id+' .selection:input').each(function(i) {
+			var iRemote = $(this).attr('data-remote-id');
+			theMap.aAlreadyLinked.push(iRemote);
+		});
 		theMap['sRemoteClass'] = theMap['class'];  // swap 'class' (defined in the form) and 'remoteClass'
 		theMap['class'] = me.sClass;
 		theMap.operation = 'searchObjectsToAdd'; // Override what is defined in the form itself
@@ -190,7 +209,7 @@ function LinksWidget(id, sClass, sAttCode, iInputId, sSuffix, bDuplicates, oWizH
 			$(' :input[name^=storedSelection]', context).each(function() {
 				if (theMap[this.name] == undefined)
 				{
-					theMap[this.name] = new Array();
+					theMap[this.name] = [];
 				}
 				theMap[this.name].push(this.value);
 				$(this).remove(); // Remove the selection for the next time the dialog re-opens
@@ -208,14 +227,13 @@ function LinksWidget(id, sClass, sAttCode, iInputId, sSuffix, bDuplicates, oWizH
 				{
 					if ( (this.name != '') && ((this.type != 'checkbox') || (this.checked)) ) 
 					{
-						//console.log(this.type);
 						arrayExpr = /\[\]$/;
 						if (arrayExpr.test(this.name))
 						{
 							// Array
 							if (theMap[this.name] == undefined)
 							{
-								theMap[this.name] = new Array();
+								theMap[this.name] = [];
 							}
 							theMap[this.name].push(this.value);
 						}
@@ -230,6 +248,7 @@ function LinksWidget(id, sClass, sAttCode, iInputId, sSuffix, bDuplicates, oWizH
 //		}
 		
 		theMap['operation'] = 'doAddObjects';
+        theMap['max_added_id'] = this.iMaxAddedId;
 		if (me.oWizardHelper == null)
 		{
 			theMap['json'] = '';
@@ -245,7 +264,6 @@ function LinksWidget(id, sClass, sAttCode, iInputId, sSuffix, bDuplicates, oWizH
 		$.post( GetAbsoluteUrlAppRoot()+'pages/ajax.render.php', theMap, 
 			function(data)
 			{
-				//console.log('Data: ' + data);
 				if (data != '')
 				{
 					$('#'+me.id+'_empty_row').hide();
@@ -262,7 +280,29 @@ function LinksWidget(id, sClass, sAttCode, iInputId, sSuffix, bDuplicates, oWizH
 		$('#dlg_'+me.id).dialog('close');
 		return false;
 	};
-	
+
+    this.OnLinkAdded = function(iAddedId, iRemote)
+    {
+        // Assumption: this identifier will be higher than the previous one
+        me.iMaxAddedId = iAddedId;
+		var sFormPrefix = me.iInputId;
+		oAdded = {};
+		oAdded['formPrefix'] = sFormPrefix;
+		oAdded['attr_' + sFormPrefix + this.sExtKeyToRemote] = iRemote;
+        me.aAdded[iAddedId] = oAdded;
+        $('#linkedset_'+me.id+' :input').off('change').on('change', function() {
+			if (!($(this).hasClass('selection'))) {
+				var oCheckbox = $(this).closest('tr').find('.selection');
+				var iLink = oCheckbox.attr('data-link-id');
+				var iUniqueId = oCheckbox.attr('data-unique-id');
+				var sAttCode = $(this).closest('.attribute-edit').attr('data-attcode');
+				var value = $(this).val();
+				return me.OnValueChange(iLink, iUniqueId, sAttCode, value);
+			}
+			return true;
+        });
+    };
+
 	this.UpdateSizes = function(event, ui)
 	{
 		var dlg = $('#dlg_'+me.id);
@@ -336,4 +376,47 @@ function LinksWidget(id, sClass, sAttCode, iInputId, sSuffix, bDuplicates, oWizH
 		});
 		return JSON.stringify(aValues);
 	};
+
+	this.OnValueChange = function(iLink, iUniqueId, sAttCode, value)
+	{
+		var sFormPrefix = me.iInputId;
+        if (iLink > 0) {
+            // Modifying an existing link
+            var oModified = me.aModified[iLink];
+            if (oModified == undefined) {
+                // Still not marked as modified
+                oModified = {};
+                oModified['formPrefix'] = sFormPrefix;
+            }
+            // Weird formatting, aligned with the output of the direct links widget (new links to be created)
+            oModified['attr_' + sFormPrefix + sAttCode] = value;
+            me.aModified[iLink] = oModified;
+        }
+        else {
+            // Modifying a newly added link - the structure should already be up to date
+            me.aAdded[iUniqueId]['attr_' + sFormPrefix + sAttCode] = value;
+        }
+	};
+
+	this.OnFormSubmit = function()
+	{
+		var oDiv = $('#linkedset_'+me.id);
+
+		var sToBeDeleted = JSON.stringify(me.aRemoved);
+		$('<input type="hidden" name="attr_'+me.sAttCode+'_tbd">').val(sToBeDeleted).appendTo(oDiv);
+
+
+		var sToBeModified = JSON.stringify(me.aModified);
+		$('<input type="hidden" name="attr_'+me.sAttCode+'_tbm">').val(sToBeModified).appendTo(oDiv);
+
+		var aToBeCreated = [];
+		me.aAdded.forEach(function(oAdded){
+			if (oAdded != null)
+			{
+				aToBeCreated.push(oAdded);
+			}
+		});
+		var sToBeCreated = JSON.stringify(aToBeCreated);
+		$('<input type="hidden" name="attr_'+me.sAttCode+'_tbc">').val(sToBeCreated).appendTo(oDiv);
+	};
 }

+ 4 - 3
pages/ajax.render.php

@@ -1,5 +1,5 @@
 <?php
-// Copyright (C) 2010-2016 Combodo SARL
+// Copyright (C) 2010-2017 Combodo SARL
 //
 //   This file is part of iTop.
 //
@@ -20,7 +20,7 @@
 /**
  * Handles various ajax requests
  *
- * @copyright   Copyright (C) 2010-2016 Combodo SARL
+ * @copyright   Copyright (C) 2010-2017 Combodo SARL
  * @license     http://opensource.org/licenses/AGPL-3.0
  */
 
@@ -530,6 +530,7 @@ try
 		$sRemoteClass = utils::ReadParam('sRemoteClass', $sClass, false, 'class');
 		$bDuplicates = (utils::ReadParam('bDuplicates', 'false') == 'false') ? false : true;
 		$sJson = utils::ReadParam('json', '', false, 'raw_data');
+		$iMaxAddedId = utils::ReadParam('max_added_id');
 		$oWizardHelper = WizardHelper::FromJSON($sJson);
 		$oObj = $oWizardHelper->GetTargetObject();
 		$oWidget = new UILinksWidget($sClass, $sAttCode, $iInputId, $sSuffix, $bDuplicates);
@@ -541,7 +542,7 @@ try
 		{
 			$oFullSetFilter = new DBObjectSearch($sRemoteClass);		
 		}
-		$oWidget->DoAddObjects($oPage, $oFullSetFilter, $oObj);	
+		$oWidget->DoAddObjects($oPage, $iMaxAddedId, $oFullSetFilter, $oObj);
 		break;
 			
 		////////////////////////////////////////////////////////////

+ 9 - 7
test/test.class.inc.php

@@ -1,5 +1,5 @@
 <?php
-// Copyright (C) 2010-2015 Combodo SARL
+// Copyright (C) 2010-2017 Combodo SARL
 //
 //   This file is part of iTop.
 //
@@ -19,7 +19,7 @@
 /**
  * Core automated tests - basics
  *
- * @copyright   Copyright (C) 2010-2015 Combodo SARL
+ * @copyright   Copyright (C) 2010-2017 Combodo SARL
  * @license     http://opensource.org/licenses/AGPL-3.0
  */
 
@@ -143,20 +143,22 @@ abstract class TestHandler
 	{
 		// Note: return false to call the default handler (stop the program if an error)
 
+		if ($errstr == 'assert()') $errno = E_USER_ERROR;
+
 		switch ($errno)
 		{
 		case E_USER_ERROR:
-			$this->ReportError($errstr);
-			//throw new ExceptionFromError("Fatal error in line $errline of file $errfile: $errstr");
+		case E_WARNING: //(assertion failed)
+			$this->ReportError("@$errline - $errstr");
 			break;
 		case E_USER_WARNING:
-			$this->ReportWarning($errstr);
+			$this->ReportWarning("@$errline - $errstr");
 			break;
 		case E_USER_NOTICE:
-			$this->ReportWarning($errstr);
+			$this->ReportWarning("@$errline - $errstr");
 			break;
 		default:
-			$this->ReportWarning("Unknown error type: [$errno] $errstr in $errfile at $errline");
+			$this->ReportWarning("@$errline - Unknown error type: [$errno] $errstr");
 			echo "Unknown error type: [$errno] $errstr in $errfile at $errline<br />\n";
 			break;
 		}

+ 197 - 2
test/testlist.inc.php

@@ -1,5 +1,5 @@
 <?php
-// Copyright (C) 2010-2016 Combodo SARL
+// Copyright (C) 2010-2017 Combodo SARL
 //
 //   This file is part of iTop.
 //
@@ -19,7 +19,7 @@
 /**
  * Core test list
  *
- * @copyright   Copyright (C) 2010-2016 Combodo SARL
+ * @copyright   Copyright (C) 2010-2017 Combodo SARL
  * @license     http://opensource.org/licenses/AGPL-3.0
  */
 
@@ -5614,3 +5614,198 @@ class TestBug689 extends TestBizModel
 		echo '<p>Well done, this is working fine! Some magic happened in the background!</p>';
 	}
 }
+
+class TestDBObjectLinkedObjects extends TestBizModel
+{
+	static public function GetName()
+	{
+		return 'DBObject Linked objects API';
+	}
+
+	static public function GetDescription()
+	{
+		return 'Add/Remove/Modify linked objects (recorded as a delta within DBObject, later recorded in DB)';
+	}
+
+	protected function DoExecute()
+	{
+		CMDBSource::Query('START TRANSACTION');
+		//CMDBSource::Query('ROLLBACK'); automatique !
+
+		////////////////////////////////////////////////////////////////////////////////
+		// Set the stage
+		//
+
+		$oTypes = new DBObjectSet(DBObjectSearch::FromOQL('SELECT NetworkDeviceType WHERE name = "Router"'));
+		$oType = $oTypes->fetch();
+
+		$oDevice1 = MetaModel::NewObject('NetworkDevice');
+		$oDevice1->Set('name', 'test device 1');
+		$oDevice1->Set('org_id', 3);
+		$oDevice1->Set('networkdevicetype_id', $oType->GetKey());
+		$oDevice1->DBInsert();
+		$iDev1 = $oDevice1->GetKey();
+
+		$oDevice2 = MetaModel::NewObject('NetworkDevice');
+		$oDevice2->Set('name', 'test device 2');
+		$oDevice2->Set('org_id', 3);
+		$oDevice2->Set('networkdevicetype_id', $oType->GetKey());
+		$oDevice2->DBInsert();
+		$iDev2 = $oDevice2->GetKey();
+
+		$oServer = MetaModel::NewObject('Server');
+		$oServer->Set('name', 'unit test linkset');
+		$oServer->Set('org_id', 3);
+		$oLinkSet = $oServer->Get('networkdevice_list');
+		$oLinkSet->AddItem(MetaModel::NewObject('lnkConnectableCIToNetworkDevice', array('networkdevice_id' => $iDev1)));
+		$oServer->Set('networkdevice_list', $oLinkSet);
+		assert($oServer->IsModified(), 'Server is modified');
+		$oServer->DBInsert();
+		$iServer = $oServer->GetKey();
+
+		$oServer = MetaModel::GetObject('Server', $iServer);
+		$oLinkSet = $oServer->Get('networkdevice_list');
+		assert($oLinkSet->Count() == 1, 'One NW Dev attached');
+		$oLink = $oLinkSet->Fetch();
+		assert($oLink->Get('networkdevice_id') == $iDev1, 'New device correctly attached');
+
+		$oLinkSet = $oServer->Get('networkdevice_list');
+		$oLinkSet->AddItem(MetaModel::NewObject('lnkConnectableCIToNetworkDevice', array('networkdevice_id' => $iDev2)));
+		$oServer->Set('networkdevice_list', $oLinkSet);
+		assert($oServer->IsModified(), 'Server is modified');
+		$oServer->DBUpdate();
+
+		$oServer = MetaModel::GetObject('Server', $iServer);
+		$oLinkSet = $oServer->Get('networkdevice_list');
+		assert($oLinkSet->Count() == 2, 'Two NW Dev attached');
+		$oNewLinkSet = clone $oLinkSet;
+		while ($oLink = $oLinkSet->Fetch())
+		{
+			$iLinkId = $oLink->Get('networkdevice_id');
+			if ($iLinkId == $iDev1)
+			{
+				$oNewLinkSet->RemoveItem($oLink->GetKey());
+			}
+			elseif ($iLinkId == $iDev2)
+			{
+				$oLink->Set('network_port', 'lePortSalut');
+				$oNewLinkSet->ModifyItem($oLink);
+			}
+		}
+		$oServer->Set('networkdevice_list', $oNewLinkSet);
+		assert($oServer->IsModified(), 'Server is modified');
+		$oServer->DBUpdate();
+
+		$oServer = MetaModel::GetObject('Server', $iServer);
+		$oLinkSet = $oServer->Get('networkdevice_list');
+		assert($oLinkSet->Count() == 1, 'One NW Dev attached');
+		$oLink = $oLinkSet->Fetch();
+		assert($oLink->Get('networkdevice_id') == $iDev2, 'Dev2 remained attached');
+		assert($oLink->Get('network_port') == 'lePortSalut', 'Port has been changed');
+	}
+}
+
+class TestDBObjectLinkedObjectsLegacy extends TestBizModel
+{
+	static public function GetName()
+	{
+		return 'DBObject Linked objects API (legacy usage)';
+	}
+
+	static public function GetDescription()
+	{
+		return 'Alter a link set by redefining the whole list of links (not recommended!)';
+	}
+
+	protected function DoExecute()
+	{
+		CMDBSource::Query('START TRANSACTION');
+		//CMDBSource::Query('ROLLBACK'); automatique !
+
+		////////////////////////////////////////////////////////////////////////////////
+		// Set the stage
+		//
+
+		$oTypes = new DBObjectSet(DBObjectSearch::FromOQL('SELECT NetworkDeviceType WHERE name = "Router"'));
+		$oType = $oTypes->fetch();
+
+		$oDevice1 = MetaModel::NewObject('NetworkDevice');
+		$oDevice1->Set('name', 'test device 1');
+		$oDevice1->Set('org_id', 3);
+		$oDevice1->Set('networkdevicetype_id', $oType->GetKey());
+		$oDevice1->DBInsert();
+		$iDev1 = $oDevice1->GetKey();
+
+		$oDevice2 = MetaModel::NewObject('NetworkDevice');
+		$oDevice2->Set('name', 'test device 2');
+		$oDevice2->Set('org_id', 3);
+		$oDevice2->Set('networkdevicetype_id', $oType->GetKey());
+		$oDevice2->DBInsert();
+		$iDev2 = $oDevice2->GetKey();
+
+		$oServer = MetaModel::NewObject('Server');
+		$oServer->Set('name', 'unit test linkset');
+		$oServer->Set('org_id', 3);
+		$oLinkSet = $oServer->Get('networkdevice_list');
+		$oNewLinkSet = DBObjectSet::FromScratch('lnkConnectableCIToNetworkDevice');
+		while ($oLink = $oLinkSet->Fetch())
+		{
+			$oNewLinkSet->AddObject($oLink);
+		}
+		$oNewLinkSet->AddObject(MetaModel::NewObject('lnkConnectableCIToNetworkDevice', array('networkdevice_id' => $iDev1)));
+		$oServer->Set('networkdevice_list', $oNewLinkSet);
+		assert($oServer->IsModified(), 'Server is modified');
+		$oServer->DBInsert();
+		$iServer = $oServer->GetKey();
+
+		$oServer = MetaModel::GetObject('Server', $iServer);
+		$oLinkSet = $oServer->Get('networkdevice_list');
+		assert($oLinkSet->Count() == 1, 'One NW Dev attached');
+		$oLink = $oLinkSet->Fetch();
+		assert($oLink->Get('networkdevice_id') == $iDev1, 'New device correctly attached');
+
+		$oNewLinkSet = DBObjectSet::FromScratch('lnkConnectableCIToNetworkDevice');
+		$oLinkSet->Rewind();
+		while ($oLink = $oLinkSet->Fetch())
+		{
+			$oNewLinkSet->AddObject($oLink);
+		}
+		$oNewLinkSet->AddObject(MetaModel::NewObject('lnkConnectableCIToNetworkDevice', array('networkdevice_id' => $iDev2)));
+		$oServer->Set('networkdevice_list', $oNewLinkSet);
+		assert($oServer->IsModified(), 'Server is modified');
+		$oServer->DBUpdate();
+
+		$oServer = MetaModel::GetObject('Server', $iServer);
+		$oLinkSet = $oServer->Get('networkdevice_list');
+		assert($oLinkSet->Count() == 2, 'Two NW Dev attached');
+		$oNewLinkSet = DBObjectSet::FromScratch('lnkConnectableCIToNetworkDevice');
+		$oServer->Set('networkdevice_list', $oNewLinkSet);
+		while ($oLink = $oLinkSet->Fetch())
+		{
+			$iLinkId = $oLink->Get('networkdevice_id');
+			if ($iLinkId == $iDev1)
+			{
+				// Remove...ie do not add it!
+			}
+			elseif ($iLinkId == $iDev2)
+			{
+				$oLink->Set('network_port', 'lePortSalut');
+				$oNewLinkSet->AddObject($oLink);
+			}
+			else
+			{
+				$oNewLinkSet->AddObject($oLink);
+			}
+		}
+		$oServer->Set('networkdevice_list', $oNewLinkSet);
+		assert($oServer->IsModified(), 'Server is modified');
+		$oServer->DBUpdate();
+
+		$oServer = MetaModel::GetObject('Server', $iServer);
+		$oLinkSet = $oServer->Get('networkdevice_list');
+		assert($oLinkSet->Count() == 1, 'One NW Dev attached');
+		$oLink = $oLinkSet->Fetch();
+		assert($oLink->Get('networkdevice_id') == $iDev2, 'Dev2 remained attached');
+		assert($oLink->Get('network_port') == 'lePortSalut', 'Port has been changed');
+	}
+}