N°2293 DBUpdate : save changed fields and corresponding previous values (#111)

* N°2293 DBUpdate : save changed fields and corresponding previous values for callbacks
* update PHPDoc
* remove m_aChanges and ListChangesUpdated() that were introduced in 2.7.0-beta
* add m_aPreviousValuesForUpdatedAttributes and ListPreviousValuesForUpdatedAttributes()

* :memo Woops forgot to change one PHPDoc

* 📝 Some more PHPDoc O:)

* 📝 Add more info in .doc README

* 📝 Well, again some PHPDoc O:)

* 📝 Replace inline @link by @see
@link are for URI, see https://docs.phpdoc.org/latest/references/phpdoc/inline-tags/link.html

.doc/README.md

@@ -23,7 +23,7 @@ Some iTop specific tags were added :
  
 ### known limitations:
 #### `@see` tags must be very specific: 
-   * always prefix class members with `ClassName::` 
+   * always prefix class members (attributes or methods) with `ClassName::` (do not use self) 
    * for methods always suffix them with `()`, 
    * do not reference variables since they are not documented. If you have to, always prefix them with `$`
  

application/applicationextension.inc.php

@@ -421,7 +421,10 @@ interface iApplicationObjectExtension
 	 * Invoked when an object is updated into the database. The method is called right <b>after</b> the object has been written to the
 	 * database.
 	 *
-	 * Changes made to the object can be get using {@link $oObject::ListChangesUpdated()}. Do not call {@link \DBObject::ListChanges} for this purpose because it will be empty as the object has already be written to DB!
+	 * Useful methods you can call on $oObject :
+	 *
+	 * * {@see DBObject::ListPreviousValuesForUpdatedAttributes()} : list of changed attributes and their values before the change
+	 * * {@see DBObject::Get()} : for a given attribute the new value that was persisted
 	 *
 	 * @param \cmdbAbstractObject $oObject The target object
 	 * @param CMDBChange|null $oChange A change context. Since 2.0 it is fine to ignore it, as the framework does maintain this information
@@ -429,7 +432,7 @@ interface iApplicationObjectExtension
 	 *
 	 * @return void
 	 *
-	 * @since 2.7.0 N°2293 can access object changes by calling {@link $oObject::ListChangesUpdated()}
+	 * @since 2.7.0 N°2293 can access object changes by calling {@see DBObject::ListPreviousValuesForUpdatedAttributes()} on $oObject
 	 */
 	public function OnDBUpdate($oObject, $oChange = null);
 

core/dbobject.class.php

@@ -83,8 +83,9 @@ abstract class DBObject implements iDisplay
 	/** @var bool true IF the object is mapped to a DB record */
 	protected $m_bIsInDB = false;
 	protected $m_iKey = null;
-	/** @var array key: attcode, value: corresponding current value (in memory, before persisting object) */
+	/** @var array attcode => value : corresponding current value (the new value passed to {@see DBObject::Set()}). Reset during {@see DBObject::DBUpdate()} */
 	private $m_aCurrValues = array();
+	/** @var array attcode => value : previous values before the {@see DBObject::Set()} call. Array is reset at the end of {@see DBObject::DBUpdate()} */
 	protected $m_aOrigValues = array();
 
 	protected $m_aExtendedData = null;
@@ -97,7 +98,8 @@ abstract class DBObject implements iDisplay
 	private $m_bDirty = false;
 
 	/**
-	 * @var boolean|null true if the object has been verified and is consistent with integrity rules. If null, then the check has to be performed again to know the status
+	 * @var boolean|null true if the object has been verified and is consistent with integrity rules.
+	 *           If null, then the check has to be performed again to know the status
 	 * @see CheckToWrite()
 	 */
 	private $m_bCheckStatus = null;
@@ -134,10 +136,11 @@ abstract class DBObject implements iDisplay
 	 */
 	protected $m_aModifiedAtt = array();
 	/**
-	 * @var array attname => currentvalue Persists changes for {@link DBUpdate}
+	 * @var array attname => value : value before the last {@see DBObject::Set()} call. Set at the beginning of {@see DBObject::DBUpdate()}.
+	 * @see DBObject::ListPreviousValuesForUpdatedAttributes() getter for this attribute
 	 * @since 2.7.0 N°2293
 	 */
-	protected $m_aChanges;
+	protected $m_aPreviousValuesForUpdatedAttributes;
 	/**
 	 * @var array Set of Synch data related to this object
 	 * <ul>
@@ -852,17 +855,13 @@ abstract class DBObject implements iDisplay
 	}
 
     /**
-     * Get the value as it was before change with Set
-     * 
-     * The original value vary according to the persisted state 
-     *   - not persisted: NULL
-     *   - persisted: the "in DB" value
-     * 
      * @param string $sAttCode
      *
-     * @return mixed|null the original value
+     * @return mixed|null the value as it was before changed with {@see DBObject::Set()}.
+     *        Returns null if the attribute wasn't changed.
      *
-     * @throws CoreException
+     * @see DBObject::$m_aOrigValues
+     * @throws CoreException if the attribute is unknown for the current object
      */
 	public function GetOriginal($sAttCode)
 	{
@@ -2237,7 +2236,7 @@ abstract class DBObject implements iDisplay
 	/**
 	 * Check if it is allowed to delete the existing object from the database
 	 *
-	 * an array of displayable error is added in {@link $m_aDeleteIssues}
+	 * an array of displayable error is added in {@see DBObject::$m_aDeleteIssues}
 	 *
      * @internal 
      *
@@ -2376,12 +2375,11 @@ abstract class DBObject implements iDisplay
 	}
 
     /**
-     * List the attributes that have been changed since the object has been loaded from the DB
-     *
      * @api
      * @api-advanced
      *
-     * @return array attname => currentvalue
+     * @return array attname => currentvalue List the attributes that have been changed using {@see DBObject::Set()}. Reset during {@see DBObject::DBUpdate()}
+     * @uses m_aCurrValues
      * @throws Exception
      */
 	public function ListChanges()
@@ -2390,27 +2388,28 @@ abstract class DBObject implements iDisplay
 		{
 			return $this->ListChangedValues($this->m_aCurrValues);
 		}
-		else
-		{
-			return $this->m_aCurrValues;
-		}
+
+		return $this->m_aCurrValues;
 	}
 
 	/**
-	 * List the changed attributes that were persisted by an update.
-	 * 
-	 * @see \DBObject::ListChanges() use DBObject::ListChanges() if your code is BEFORE the update 
-	 * 
-	 * @return array
+	 * @api
+	 * @api-advanced
+	 *
+	 * @return array attname => value : value that was present before the last {@see DBObject::Set()} call.
+	 *       This array is set at the beginning of {@see DBObject::DBpdate()} using {@see DBObject::InitPreviousValuesForUpdatedAttributes()}
+	 * @uses m_aPreviousValuesForUpdatedAttributes
+	 * @see DBObject::ListChanges() use this other method if your code is BEFORE the update
+	 * @since 2.7.0 N°2293
 	 */
-	public function ListChangesUpdated()
+	public function ListPreviousValuesForUpdatedAttributes()
 	{
-		if (empty($this->m_aChanges))
+		if (empty($this->m_aPreviousValuesForUpdatedAttributes))
 		{
 			return array();
 		}
 
-		return $this->m_aChanges;
+		return $this->m_aPreviousValuesForUpdatedAttributes;
 	}
 
 	
@@ -2660,7 +2659,7 @@ abstract class DBObject implements iDisplay
 	 *
 	 * @return int key of the newly created object
 	 * @throws \ArchivedObjectException
-	 * @throws \CoreCannotSaveObjectException if {@link CheckToWrite()} returns issues
+	 * @throws \CoreCannotSaveObjectException if {@see DBObject::CheckToWrite()} returns issues
 	 * @throws \CoreException
 	 * @throws \CoreUnexpectedValue
 	 * @throws \CoreWarning
@@ -2945,8 +2944,8 @@ abstract class DBObject implements iDisplay
     /**
      * @internal
      *
-     * @deprecated 2.7.0 N°2361 simply use {@link DBInsert} instead, that will automatically create and persist a CMDBChange object.
-     *    If you need to persist your own, call {@link CMDBObject::SetCurrentChange} before.
+     * @deprecated 2.7.0 N°2361 simply use {@see DBObject::DBInsert()} instead, that will automatically create and persist a CMDBChange object.
+     *    If you need to persist your own, call {@see CMDBObject::SetCurrentChange()} before.
      *
      * @param CMDBChange $oChange
      *
@@ -2962,8 +2961,8 @@ abstract class DBObject implements iDisplay
     /**
      * @internal
      *
-     * @deprecated 2.7.0 N°2361 simply use {@link DBInsertNoReload} instead, that will automatically create and persist a CMDBChange object.
-     *    If you need to persist your own, call {@link CMDBObject::SetCurrentChange} before.
+     * @deprecated 2.7.0 N°2361 simply use {@see DBObject::DBInsertNoReload()} instead, that will automatically create and persist a CMDBChange object.
+     *    If you need to persist your own, call {@see CMDBObject::SetCurrentChange()} before.
      *
      * @param CMDBChange $oChange
      *
@@ -3025,13 +3024,14 @@ abstract class DBObject implements iDisplay
 	/**
 	 * Update an object in DB
 	 *
-     * @api
-     * @see DBWrite
-     * 
+	 * @api
+	 * @see DBObject::DBWrite()
+	 *
 	 * @return int object key
-     *
+	 *
 	 * @throws \CoreException
 	 * @throws \CoreCannotSaveObjectException if CheckToWrite() returns issues
+	 * @throws \Exception
 	 */
 	public function DBUpdate()
 	{
@@ -3048,7 +3048,8 @@ abstract class DBObject implements iDisplay
 		}
 		$aUpdateReentrance[$sKey] = true;
 
-		$this->m_aChanges = array(); // reset attribute to avoid stack collisions
+		$this->InitPreviousValuesForUpdatedAttributes();
+
 		try
 		{
 			$this->DoComputeValues();
@@ -3201,7 +3202,9 @@ abstract class DBObject implements iDisplay
 					$this->DBWriteLinks();
 					$this->WriteExternalAttributes();
 
-					$this->m_aChanges = $this->ListChanges(); // N°2293 save changes for use in user callbacks
+					// following lines are resetting changes (so after this {@see DBObject::ListChanges()} won't return changes anymore)
+					// new values are already in the object (call {@see DBObject::Get()} to get them)
+					// call {@see DBObject::ListPreviousValuesForUpdatedAttributes()} to get changed fields and previous values
 					$this->m_bDirty = false;
 					$this->m_aTouchedAtt = array();
 					$this->m_aModifiedAtt = array();
@@ -3305,12 +3308,36 @@ abstract class DBObject implements iDisplay
 		return $this->m_iKey;
 	}
 
+	/**
+	 * @internal
+	 * Save updated fields previous values for {@see DBObject::DBUpdate()} callbacks
+	 * @see DBObject::ListPreviousValuesForUpdatedAttributes() to get the data in the callbacks
+	 * @uses ListChanges
+	 * @uses m_aOrigValues
+	 * @uses m_aPreviousValuesForUpdatedAttributes
+	 * @since 2.7.0 N°2293
+	 * @throws \Exception
+	 */
+	private function InitPreviousValuesForUpdatedAttributes()
+	{
+		$aChanges= $this->ListChanges();
+		if (empty($aChanges))
+		{
+			$this->m_aPreviousValuesForUpdatedAttributes = array();
+			return;
+		}
+
+		$aPreviousValuesForUpdatedAttributes = array_intersect_key($this->m_aOrigValues, $aChanges);
+
+		$this->m_aPreviousValuesForUpdatedAttributes = $aPreviousValuesForUpdatedAttributes;
+	}
+
     /**
      *
      * @internal
      *
-     * @deprecated 2.7.0 N°2361 simply use {@link DBUpdate} instead, that will automatically create and persist a CMDBChange object.
-     *    If you need to persist your own, call {@link CMDBObject::SetCurrentChange} before.
+     * @deprecated 2.7.0 N°2361 simply use {@see DBObject::DBUpdate()} instead, that will automatically create and persist a CMDBChange object.
+     *    If you need to persist your own, call {@see CMDBObject::SetCurrentChange()} before.
      *
      * @param CMDBChange $oChange
      *
@@ -3581,8 +3608,8 @@ abstract class DBObject implements iDisplay
     /**
      * @internal
      *
-     * @deprecated 2.7.0 N°2361 simply use {@link DBDelete} instead.
-     *    If you need to persist your own, call {@link CMDBObject::SetCurrentChange} before.
+     * @deprecated 2.7.0 N°2361 simply use {@see DBObject::DBDelete()} instead.
+     *    If you need to persist your own, call {@see CMDBObject::SetCurrentChange()} before.
      *
      * @param CMDBChange $oChange
      * @param boolean $bSkipStrongSecurity
@@ -4135,13 +4162,15 @@ abstract class DBObject implements iDisplay
 	}
 
     /**
-     * This method is called after the object is updated into DB. You can get changes using @link m_aChanges}.
-     *
-     * Warning : do not use {@link ListChanges} as it will return an empty array.
-     *
      * @overwritable-hook You can extend this method in order to provide your own logic.
      *
-     * @since 2.7.0 N°2293 can access object changes using {@link m_aChanges}
+     * This method is called after the object is updated into DB, and just before the {@see DBObject::Reload()} call.
+     *
+     * Warning : do not use {@see DBObject::ListChanges()} as it will return an empty array !
+     * Use instead {@see DBObject::ListPreviousValuesForUpdatedAttributes()} to get modified fields and their previous values,
+     * and {@see DBObject::Get()} to get the persisted value for a given attribute.
+     *
+     * @since 2.7.0 N°2293 can access object changes by calling {@see DBObject::ListPreviousValuesForUpdatedAttributes()}
      */
 	protected function AfterUpdate()
 	{