N°4342 - Improve generic bulk deletion function with memory limit han…

…dling (#321)
Combodo · Feb 28, 2024 · baa05ba · baa05ba
1 parent e7b493d
commit baa05ba
Show file tree

Hide file tree

Showing 3 changed files with 82 additions and 34 deletions.
diff --git a/core/config.class.inc.php b/core/config.class.inc.php
@@ -447,14 +447,14 @@ class Config
 			'show_in_conf_sample' => true,
 		],
 		'export_pdf_font' => [ // @since 2.7.0 PR #49 / N°1947
-			'type' => 'string',
-			'description' => 'Font used when generating a PDF file',
-			'default' => 'DejaVuSans', // DejaVuSans is a UTF-8 Unicode font, embedded in the TCPPDF lib we're using
-			// Standard PDF fonts like helvetica or times newroman are NOT Unicode
-			// A new DroidSansFallback can be used to improve CJK support (se PR #49)
-			'value' => '',
-			'source_of_value' => '',
-			'show_in_conf_sample' => false,
+		                       'type'                => 'string',
+		                       'description'         => 'Font used when generating a PDF file',
+		                       'default'             => 'DejaVuSans', // DejaVuSans is a UTF-8 Unicode font, embedded in the TCPPDF lib we're using
+		                       // Standard PDF fonts like helvetica or times newroman are NOT Unicode
+		                       // A new DroidSansFallback can be used to improve CJK support (se PR #49)
+		                       'value'               => '',
+		                       'source_of_value'     => '',
+		                       'show_in_conf_sample' => false,
 		],
 		'access_mode' => [
 			'type' => 'integer',
@@ -1119,6 +1119,14 @@ class Config
 			'source_of_value' => '',
 			'show_in_conf_sample' => false,
 		],
+		'purge_data.max_chunk_size' => [
+			'type' => 'integer',
+			'description' => 'Maximum number of items deleted per loop. Used in function MetaModel::PurgeData',
+			'default' => 1000,
+			'value' => 1000,
+			'source_of_value' => '',
+			'show_in_conf_sample' => false,
+		],
 		'max_history_length' => [
 			'type' => 'integer',
 			'description' => 'Maximum length of the history table (in the "History" tab on each object) before it gets truncated. Latest modifications are displayed first.',
@@ -1324,9 +1332,9 @@ class Config
 		'draft_attachments_lifetime' => [
 			'type'                => 'integer',
 			'description'         => 'Lifetime (in seconds) of drafts\' attachments and inline images: after this duration, the garbage collector will delete them.',
-			'default' => 86400,
-			'value' => '',
-			'source_of_value' => '',
+			'default'             => 86400,
+			'value'               => '',
+			'source_of_value'     => '',
 			'show_in_conf_sample' => false,
 		],
 		'date_and_time_format' => [
@@ -1882,6 +1890,7 @@ public function IsCustomValue(string $sPropCode): bool
 	 * @var integer Number of seconds between two reloads of the display (standard)
 	 */
 	protected $m_iStandardReloadInterval;
+
 	/**
 	 * @var integer Number of seconds between two reloads of the display (fast)
 	 */
@@ -2553,9 +2562,9 @@ public function WriteToFile($sFileName = '')
 
 			// Old fashioned integer settings
 			$aIntValues = array(
-				'fast_reload_interval' => $this->m_iFastReloadInterval,
-				'max_display_limit' => $this->m_iMaxDisplayLimit,
-				'min_display_limit' => $this->m_iMinDisplayLimit,
+				'fast_reload_interval'     => $this->m_iFastReloadInterval,
+				'max_display_limit'        => $this->m_iMaxDisplayLimit,
+				'min_display_limit'        => $this->m_iMinDisplayLimit,
 				'standard_reload_interval' => $this->m_iStandardReloadInterval,
 			);
 			foreach ($aIntValues as $sKey => $iValue)

diff --git a/core/metamodel.class.php b/core/metamodel.class.php
@@ -7168,32 +7168,45 @@ public static function BulkUpdate(DBObjectSearch $oFilter, array $aValues)
 	 */
 	public static function PurgeData($oFilter)
 	{
+		$iMaxChunkSize = MetaModel::GetConfig()->Get('purge_data.max_chunk_size');
 		$sTargetClass = $oFilter->GetClass();
-		$oSet = new DBObjectSet($oFilter);
-		$oSet->OptimizeColumnLoad(array($sTargetClass => array('finalclass')));
-		$aIdToClass = $oSet->GetColumnAsArray('finalclass', true);
+		$iNbIdsDeleted = 0;
+		$bExecuteQuery = true;
+
+		// This loop allows you to delete objects in batches of $iMaxChunkSize elements
+		while ($bExecuteQuery) {
+			$oSet = new DBObjectSet($oFilter);
+			$oSet->SetLimit($iMaxChunkSize);
+			$oSet->OptimizeColumnLoad(array($sTargetClass => array('finalclass')));
+			$aIdToClass = $oSet->GetColumnAsArray('finalclass', true);
+
+			$aIds = array_keys($aIdToClass);
+			$iNbIds = count($aIds);
+			if ($iNbIds > 0) {
+				$aQuotedIds = CMDBSource::Quote($aIds);
+				$sIdList = implode(',', $aQuotedIds);
+				$aTargetClasses = array_merge(
+					self::EnumChildClasses($sTargetClass, ENUM_CHILD_CLASSES_ALL),
+					self::EnumParentClasses($sTargetClass, ENUM_PARENT_CLASSES_EXCLUDELEAF)
+				);
+				foreach ($aTargetClasses as $sSomeClass) {
+					$sTable = MetaModel::DBGetTable($sSomeClass);
+					$sPKField = MetaModel::DBGetKey($sSomeClass);
 
-		$aIds = array_keys($aIdToClass);
-		if (count($aIds) > 0)
-		{
-			$aQuotedIds = CMDBSource::Quote($aIds);
-			$sIdList = implode(',', $aQuotedIds);
-			$aTargetClasses = array_merge(
-				self::EnumChildClasses($sTargetClass, ENUM_CHILD_CLASSES_ALL),
-				self::EnumParentClasses($sTargetClass, ENUM_PARENT_CLASSES_EXCLUDELEAF)
-			);
-			foreach($aTargetClasses as $sSomeClass)
-			{
-				$sTable = MetaModel::DBGetTable($sSomeClass);
-				$sPKField = MetaModel::DBGetKey($sSomeClass);
+					$sDeleteSQL = "DELETE FROM `$sTable` WHERE `$sPKField` IN ($sIdList)";
+					CMDBSource::DeleteFrom($sDeleteSQL);
+				}
+				$iNbIdsDeleted += $iNbIds;
+			}
 
-				$sDeleteSQL = "DELETE FROM `$sTable` WHERE `$sPKField` IN ($sIdList)";
-				CMDBSource::DeleteFrom($sDeleteSQL);
+			// stop loop if query returned fewer objects than  $iMaxChunkSize. In this case, all objects have been deleted.
+			if ($iNbIds < $iMaxChunkSize) {
+				$bExecuteQuery = false;
 			}
 		}
-		return count($aIds);
-	}
 
+		return $iNbIdsDeleted;
+	}
 	// Links
 	//
 	//

diff --git a/tests/php-unit-tests/unitary-tests/core/MetaModelTest.php b/tests/php-unit-tests/unitary-tests/core/MetaModelTest.php
@@ -5,6 +5,7 @@
 
 use Combodo\iTop\Test\UnitTest\ItopDataTestCase;
 use CoreException;
+use DBObjectSearch;
 use MetaModel;
 
 /**
@@ -456,6 +457,31 @@ public function IsObjectInDBProvider(): array
 			'Non existing person' => [10, false],
 		];
 	}
+
+    /**
+     * @return void
+     * @throws CoreException
+     * @throws \OQLException
+     */
+    public function testPurgeData(){
+        // Set max_chunk_size to 2 (default 1000) to test chunk deletion with only 10 items
+        $oConfig = MetaModel::GetConfig();
+        $oConfig->Set('purge_data.max_chunk_size', 2);
+        MetaModel::SetConfig($oConfig);
+
+        $aPkPerson = [];
+        for ($i=0; $i < 10; $i++) {
+            $oPerson = $this->CreatePerson($i, 1);
+            $sClass = get_class($oPerson);
+            $aPkPerson[] = $oPerson->GetKey();
+        }
+
+        $sDeleteOQL = 'SELECT '.$sClass.' WHERE id IN ('.implode(',', $aPkPerson).')';
+        $oFilter = DBObjectSearch::FromOQL($sDeleteOQL);
+
+        $iNbDelete = MetaModel::PurgeData($oFilter);
+        $this->assertEquals($iNbDelete, 10, 'MetaModel::PurgeData must delete 10 objects per batch of 2 items');
+    }
 }
 
 abstract class Wizzard