cache returned objects for next round + add jeffrey test

core/cmdbsource.class.inc.php

@@ -537,6 +537,12 @@ class CMDBSource
 	 */
 	public static function Query($sSQLQuery)
 	{
+		if (self::$sRaisesExceptionMsgWhenSqlQuery) {
+			$e = new \Exception(self::$sRaisesExceptionMsgWhenSqlQuery);
+			\IssueLog::Error(__METHOD__, null, [$e->getTraceAsString()]);
+			throw $e;
+		}
+
 		if (preg_match('/^START TRANSACTION;?$/i', $sSQLQuery)) {
 			self::StartTransaction();
 
@@ -556,6 +562,13 @@ class CMDBSource
 		return self::DBQuery($sSQLQuery);
 	}
 
+	public static ?string $sRaisesExceptionMsgWhenSqlQuery = null;
+
+	public static function TriggerExceptionWhenSqlQuery(?string $sMsg)
+	{
+		self::$sRaisesExceptionMsgWhenSqlQuery = $sMsg;
+	}
+
 	/**
 	 * Send the query directly to the DB. **Be extra cautious with this !**
 	 *

core/dbobjectset.class.php

@@ -122,9 +122,7 @@ class DBObjectSet implements iDBObjectSetIterator
 	 */
 	public function __destruct()
 	{
-		if (is_object($this->m_oSQLResult)) {
-			$this->m_oSQLResult->free();
-		}
+		$this->Free();
 	}
 
 	/**
@@ -711,11 +709,8 @@ class DBObjectSet implements iDBObjectSetIterator
 
 		$sSQL = $this->_makeSelectQuery($this->m_aAttToLoad);
 
-		if (is_object($this->m_oSQLResult)) {
-			// Free previous resultset if any
-			$this->m_oSQLResult->free();
-			$this->m_oSQLResult = null;
-		}
+		// Free previous resultset if any
+		$this->Free();
 
 		try {
 			$oKPI = new ExecutionKPI();
@@ -871,23 +866,7 @@ class DBObjectSet implements iDBObjectSetIterator
 	 */
 	public function CountExceeds($iLimit)
 	{
-		if (is_null($this->m_iNumTotalDBRows)) {
-			$oKPI = new ExecutionKPI();
-			$sSQL = $this->m_oFilter->MakeSelectQuery([], $this->m_aArgs, null, null, $iLimit + 2, 0, true);
-			$resQuery = CMDBSource::Query($sSQL);
-			$sOQL = $this->GetPseudoOQL($this->m_oFilter, [], $iLimit + 2, 0, true);
-			$oKPI->ComputeStats('OQL Query Exec', $sOQL);
-			if ($resQuery) {
-				$aRow = CMDBSource::FetchArray($resQuery);
-				$iCount = intval($aRow['COUNT']);
-				CMDBSource::FreeResult($resQuery);
-			} else {
-				$iCount = 0;
-			}
-		} else {
-			$iCount = $this->m_iNumTotalDBRows;
-		}
-
+		$iCount = $this->CountWithLimit($iLimit);
 		return ($iCount > $iLimit);
 	}
 
@@ -913,8 +892,8 @@ class DBObjectSet implements iDBObjectSetIterator
 			$oKPI->ComputeStats('OQL Query Exec', $sOQL);
 			if ($resQuery) {
 				$aRow = CMDBSource::FetchArray($resQuery);
-				CMDBSource::FreeResult($resQuery);
 				$iCount = intval($aRow['COUNT']);
+				CMDBSource::FreeResult($resQuery);
 			} else {
 				$iCount = 0;
 			}
@@ -935,6 +914,14 @@ class DBObjectSet implements iDBObjectSetIterator
 		return $this->m_iNumLoadedDBRows + count($this->m_aAddedObjects);
 	}
 
+	private function Free()
+	{
+		if (is_object($this->m_oSQLResult)) {
+			CMDBSource::FreeResult($this->m_oSQLResult);
+			$this->m_oSQLResult = null;
+		}
+	}
+
 	/**
 	 * Fetch an object (with the given class alias) at the current position in the set and move the cursor to the next position.
 	 *
@@ -955,6 +942,7 @@ class DBObjectSet implements iDBObjectSetIterator
 		}
 
 		if ($this->m_iCurrRow >= $this->CountLoaded()) {
+			$this->Free();
 			return null;
 		}
 
@@ -962,7 +950,9 @@ class DBObjectSet implements iDBObjectSetIterator
 			$sRequestedClassAlias = $this->m_oFilter->GetClassAlias();
 		}
 
-		if ($this->m_iCurrRow < $this->m_iNumLoadedDBRows) {
+		if ($this->m_iCurrRow < count($this->m_aCacheObj)) {
+			$oRetObj = $this->m_aCacheObj[$this->m_iCurrRow][$sRequestedClassAlias];
+		} else if ($this->m_iCurrRow < $this->m_iNumLoadedDBRows) {
 			// Pick the row from the database
 			$aRow = CMDBSource::FetchArray($this->m_oSQLResult);
 			foreach ($this->m_oFilter->GetSelectedClasses() as $sClassAlias => $sClass) {
@@ -972,6 +962,7 @@ class DBObjectSet implements iDBObjectSetIterator
 					} else {
 						try {
 							$oRetObj = MetaModel::GetObjectByRow($sClass, $aRow, $sClassAlias, $this->m_aAttToLoad, $this->m_aExtendedDataSpec);
+							$this->m_aCacheObj[$this->m_iCurrRow] = [$sRequestedClassAlias => $oRetObj];
 						} catch (CoreException $e) {
 							$this->m_iCurrRow++;
 							$oRetObj = $this->Fetch($sRequestedClassAlias);
@@ -988,6 +979,8 @@ class DBObjectSet implements iDBObjectSetIterator
 		return $oRetObj;
 	}
 
+	private $m_aCacheObj = [];
+
 	/**
 	 * Fetch the whole row of objects (if several classes have been specified in the query) and move the cursor to the next position
 	 *
@@ -1006,21 +999,29 @@ class DBObjectSet implements iDBObjectSetIterator
 		}
 
 		if ($this->m_iCurrRow >= $this->CountLoaded()) {
+			$this->Free();
 			return null;
 		}
 
-		if ($this->m_iCurrRow < $this->m_iNumLoadedDBRows) {
+		if ($this->m_iCurrRow < count($this->m_aCacheObj)) {
+			$aRetObjects = $this->m_aCacheObj[$this->m_iCurrRow];
+		} else if ($this->m_iCurrRow < $this->m_iNumLoadedDBRows) {
 			// Pick the row from the database
 			$aRow = CMDBSource::FetchArray($this->m_oSQLResult);
 			$aRetObjects = [];
 			foreach ($this->m_oFilter->GetSelectedClasses() as $sClassAlias => $sClass) {
-				if (is_null($aRow[$sClassAlias.'id'])) {
-					$oObj = null;
-				} else {
-					$oObj = MetaModel::GetObjectByRow($sClass, $aRow, $sClassAlias, $this->m_aAttToLoad, $this->m_aExtendedDataSpec);
+				$oObj = null;
+				if (!is_null($aRow[$sClassAlias.'id'])) {
+					try {
+						$oObj = MetaModel::GetObjectByRow($sClass, $aRow, $sClassAlias, $this->m_aAttToLoad, $this->m_aExtendedDataSpec);
+					}
+					catch (CoreException $e) {
+					}
 				}
 				$aRetObjects[$sClassAlias] = $oObj;
 			}
+
+			$this->m_aCacheObj[$this->m_iCurrRow] = $aRetObjects;
 		} else {
 			// Pick the row from the objects added *in memory*
 			$aRetObjects = [];
@@ -1063,6 +1064,10 @@ class DBObjectSet implements iDBObjectSetIterator
 			$this->Load();
 		}
 
+		if (is_null($this->m_oSQLResult)) {
+			return;
+		}
+
 		$this->m_iCurrRow = min($iRow, $this->Count());
 		if ($this->m_iCurrRow < $this->m_iNumLoadedDBRows) {
 			$this->m_oSQLResult->data_seek($this->m_iCurrRow);
@@ -1236,7 +1241,7 @@ class DBObjectSet implements iDBObjectSetIterator
 	 *
 	 * @throws \CoreException
 	 */
-	public function HasSameContents(DBObjectSet $oObjectSet, $aExcludeColumns = [])
+	public function HasSameContents(DBObjectSet $oObjectSet, $aExcludeColumns = []): bool
 	{
 		$oComparator = new DBObjectSetComparator($this, $oObjectSet, $aExcludeColumns);
 		return $oComparator->SetsAreEquivalent();

core/userrights.class.inc.php

@@ -415,12 +415,7 @@ abstract class User extends cmdbAbstractObject
 							$this->m_aCheckIssues[] = Dict::S('Class:User/Error:CurrentProfilesHaveInsufficientRights');
 						}
 						$oAddon->ResetCache();
-
-						if (is_null($aCurrentProfiles)) {
-							Session::IsSet('profile_list');
-						} else {
-							Session::Set('profile_list', $aCurrentProfiles);
-						}
+						Session::Set('profile_list', $aCurrentProfiles);
 					}
 					// Prevent an administrator to remove their own admin profile
 					if (UserRights::IsAdministrator($this)) {

tests/php-unit-tests/unitary-tests/core/DBObject/DBObjectSetTest.php

@@ -0,0 +1,138 @@
+<?php
+
+use Combodo\iTop\Test\UnitTest\ItopDataTestCase;
+
+class DBObjectSetTest extends ItopDataTestCase
+{
+	public const USE_TRANSACTION = true;
+
+	protected function setUp(): void
+	{
+		parent::setUp(); // TODO: Change the autogenerated stub
+		$this->CreateURs();
+	}
+
+	protected function tearDown(): void
+	{
+		CMDBSource::TriggerExceptionWhenSqlQuery(null);
+		parent::tearDown(); // TODO: Change the autogenerated stub
+	}
+
+	private function CreateURs()
+	{
+		$this->CreateTestOrganization();
+		for ($i = 0; $i < 10; $i++) {
+			$this->CreateTicket($i);
+		}
+	}
+
+	public function testCount()
+	{
+		$oSearch = DBObjectSearch::FromOQL_AllData("SELECT UserRequest");
+		$oSet = new DBObjectSet($oSearch);
+
+		$iCount = $oSet->Count();
+		$oSet->Fetch();
+		$this->assertEquals($iCount, $oSet->Count());
+		$this->assertEquals($iCount, $oSet->CountWithLimit(0));
+		$this->assertTrue($oSet->CountExceeds(0));
+
+		//no DB SQL query: exception will be raised after here
+		CMDBSource::TriggerExceptionWhenSqlQuery(__METHOD__.' :'.__LINE__);
+		$this->assertEquals($iCount, $oSet->Count(), 'should use cache and not call DB again');
+	}
+
+	public function testRewind()
+	{
+		$oSearch = DBObjectSearch::FromOQL_AllData("SELECT UserRequest");
+		$oSet = new DBObjectSet($oSearch);
+
+		while ($oObj = $oSet->Fetch()) {
+			$this->assertNotEquals(0, $oObj->GetKey());
+		}
+
+		//no DB SQL query: exception will be raised after here
+		CMDBSource::TriggerExceptionWhenSqlQuery(__METHOD__.' :'.__LINE__);
+		$oSet->Rewind();
+		while ($oObj = $oSet->Fetch()) {
+			$this->assertNotEquals(0, $oObj->GetKey());
+		}
+	}
+
+	public function testDBObjectSetComparator()
+	{
+		$oSearch = DBObjectSearch::FromOQL_AllData("SELECT UserRequest");
+		$DBObjectSet1 = new DBObjectSet($oSearch);
+		$DBObjectSet3 = new DBObjectSet($oSearch);
+		$oDBObjectSetComparator = new DBObjectSetComparator($DBObjectSet1, $DBObjectSet3);
+		$this->assertTrue($oDBObjectSetComparator->SetsAreEquivalent());
+	}
+
+	public function testDBObjectSetComparator_CheckCache()
+	{
+		$oSearch = DBObjectSearch::FromOQL_AllData("SELECT UserRequest");
+		$DBObjectSet1 = new DBObjectSet($oSearch);
+		$DBObjectSet3 = new DBObjectSet($oSearch);
+		$oDBObjectSetComparator = new DBObjectSetComparator($DBObjectSet1, $DBObjectSet3);
+		$this->assertTrue($oDBObjectSetComparator->SetsAreEquivalent());
+
+		$sMsg = __METHOD__.' :'.__LINE__;
+		//no DB SQL query: exception will be raised after here
+		CMDBSource::TriggerExceptionWhenSqlQuery($sMsg);
+		$oDBObjectSetComparator = new DBObjectSetComparator($DBObjectSet1, $DBObjectSet3);
+		$this->assertTrue($oDBObjectSetComparator->SetsAreEquivalent());
+
+		$oDBObjectSetComparator = new DBObjectSetComparator($DBObjectSet1, new DBObjectSet($oSearch));
+		$this->expectExceptionMessage($sMsg, "should call DB again this time");
+		$this->assertTrue($oDBObjectSetComparator->SetsAreEquivalent());
+	}
+
+	public static function JeffreyProvider()
+	{
+		return [
+			'basic'     => [false],
+			'opt trick' => [true],
+		];
+	}
+
+	/**
+	 * @dataProvider JeffreyProvider
+	 */
+	public function testJeffrey(bool $bTrick)
+	{
+		echo '<p>-------</p>';
+
+		$this->doesNotPerformAssertions();
+		$iMax = 100;
+
+
+		$oFilter = DBObjectSearch::FromOQL_AllData('SELECT UserRequest');
+		$oSet = new DBObjectSet($oFilter);
+		$oSet->OptimizeColumnLoad([
+			'UserRequest' => ['ref', 'status', 'title'],
+		]);
+
+		if ($bTrick) {
+			$oNewSet = DBObjectSet::FromScratch($oSet->GetClass());
+			while ($oObj = $oSet->Fetch()) {
+				$oNewSet->AddObject($oObj);
+			}
+			$oSet = $oNewSet;
+		}
+
+		echo '<p>Start: '.date('Y-m-d H:i:s').'</p>';
+		$i = 0;
+		while ($i < $iMax) {
+			$oSet->Rewind();
+			while ($oObj = $oSet->Fetch()) {
+				// Do nothing
+				$s = $oObj->Get('title');
+			}
+
+			$i += 1;
+		}
+
+		$peak = memory_get_peak_usage(true) / 1000000;
+		echo '<p>End: '.date('Y-m-d H:i:s').'</p><p>Peak memory: '.$peak.'</p> \n';
+	}
+}