WIP

2026-08-22 22:28:19 +02:00 · 2026-06-08 18:41:01 +02:00
parent 90df74f397
commit b2329939b6
5 changed files with 177 additions and 164 deletions
--- a/dictionaries/en.dictionary.itop.core.php
+++ b/dictionaries/en.dictionary.itop.core.php
@@ -217,7 +217,7 @@ Operators:<br/>
 	'Core:Context=CRON' => 'cron',
 	'Core:Context=GUI:Portal' => 'Portal',

-	'Core:GetQuota:Error' => 'Error while getting %1$s quota',
+	'Core:GetCountingUsers:Error' => 'Error while getting %1$s quota',
 	'Core:ConsoleUsers' => 'console users',
 	'Core:DisabledUsers' => 'disabled users',
 	'Core:PortalUsers' => 'portal users',
--- a/lib/composer/autoload_classmap.php
+++ b/lib/composer/autoload_classmap.php
@@ -651,7 +651,7 @@ return array(
    'Combodo\\iTop\\SessionTracker\\SessionGC' => $baseDir . '/sources/SessionTracker/SessionGC.php',
    'Combodo\\iTop\\SessionTracker\\SessionHandler' => $baseDir . '/sources/SessionTracker/SessionHandler.php',
    'Combodo\\iTop\\SessionTracker\\iSessionHandlerExtension' => $baseDir . '/sources/SessionTracker/iSessionHandlerExtension.php',
-    'Combodo\\iTop\\Users\\ITopUserQuotaRepository' => $baseDir . '/sources/Users/ITopUserQuotaRepository.php',
+    'Combodo\\iTop\\Users\\ITopUserCountingRepository' => $baseDir . '/sources/Users/ITopUserCountingRepository.php',
    'CompileCSSService' => $baseDir . '/application/compilecssservice.class.inc.php',
    'Composer\\InstalledVersions' => $vendorDir . '/composer/InstalledVersions.php',
    'Config' => $baseDir . '/core/config.class.inc.php',
--- a/lib/composer/autoload_static.php
+++ b/lib/composer/autoload_static.php
@@ -1052,7 +1052,7 @@ class ComposerStaticInitfc0e9e9dea11dcbb6272414776c30685
        'Combodo\\iTop\\SessionTracker\\SessionGC' => __DIR__ . '/../..' . '/sources/SessionTracker/SessionGC.php',
        'Combodo\\iTop\\SessionTracker\\SessionHandler' => __DIR__ . '/../..' . '/sources/SessionTracker/SessionHandler.php',
        'Combodo\\iTop\\SessionTracker\\iSessionHandlerExtension' => __DIR__ . '/../..' . '/sources/SessionTracker/iSessionHandlerExtension.php',
-        'Combodo\\iTop\\Users\\ITopUserQuotaRepository' => __DIR__ . '/../..' . '/sources/Users/ITopUserQuotaRepository.php',
+        'Combodo\\iTop\\Users\\ITopUserCountingRepository' => __DIR__ . '/../..' . '/sources/Users/ITopUserCountingRepository.php',
        'CompileCSSService' => __DIR__ . '/../..' . '/application/compilecssservice.class.inc.php',
        'Composer\\InstalledVersions' => __DIR__ . '/..' . '/composer/InstalledVersions.php',
        'Config' => __DIR__ . '/../..' . '/core/config.class.inc.php',
--- a/sources/Users/ITopUserCountingRepository.php
+++ b/sources/Users/ITopUserCountingRepository.php
@@ -22,13 +22,13 @@ use UserRights;
 /**
 *
 */
-class ITopUserQuotaRepository
+class ITopUserCountingRepository
 {
 	/**
-	 * @param string $sExcludedUsers
-	 * @param string $sExcludedProfiles
+	 * @param array $aExcludedUsers
+	 * @param array $aExcludedProfiles
 	 * @param bool $bAllData
-	 * @param string $sExcludedFinalClasses
+	 * @param array $aExcludedFinalClasses
 	 *
 	 * @return array
 	 * @throws CoreException
@@ -37,23 +37,26 @@ class ITopUserQuotaRepository
 	 * @throws MySQLException
 	 * @throws Exception
 	 */
-	public function GetConsoleUsers(string $sExcludedUsers = '', string $sExcludedProfiles = '', bool $bAllData = true, string $sExcludedFinalClasses = 'UserToken, UserRemoteSaaS'): array
+	public function GetConsoleUsers(array $aExcludedUsers = [], array $aExcludedProfiles = [], bool $bAllData = true, array $aExcludedFinalClasses = ['UserToken', 'UserRemoteSaaS']): array
 	{
-		$sOQLInQuotaUser = "
-        SELECT User AS u
-        WHERE u.status != 'disabled'
-        AND u.login NOT IN ('$sExcludedUsers')
-        AND u.finalclass != ' $sExcludedFinalClasses '
-        AND id NOT IN (
-            SELECT User AS uex
-            JOIN URP_UserProfile AS uup ON uup.userid = uex.id
-            JOIN URP_Profiles AS up ON uup.profileid = up.id
-            WHERE up.name IN ('$sExcludedProfiles'))
-        ";
+		$sExcludedUsers = $this->ArrayToOQLStringParameter($aExcludedUsers);
+		$sExcludedProfiles = $this->ArrayToOQLStringParameter($aExcludedProfiles);
+		$sExcludedFinalClasses = $this->ArrayToOQLStringParameter($aExcludedFinalClasses);
+
+		$sOQLUserConsole = "
+		SELECT User AS u
+		WHERE u.status != 'disabled'
+		AND u.login NOT IN ($sExcludedUsers)
+		AND u.finalclass NOT IN ($sExcludedFinalClasses)
+		AND id NOT IN (
+			SELECT User AS uex
+			JOIN URP_UserProfile AS uup ON uup.userid = uex.id
+			JOIN URP_Profiles AS up ON uup.profileid = up.id
+			WHERE up.name IN ($sExcludedProfiles))
+	";
 		try {
-			$oFilter = $bAllData ? DBObjectSearch::FromOQL_AllData($sOQLInQuotaUser) : DBObjectSearch::FromOQL($sOQLInQuotaUser);
-		}
-		catch (Exception $e) {
+			$oFilter = $bAllData ? DBObjectSearch::FromOQL_AllData($sOQLUserConsole) : DBObjectSearch::FromOQL($sOQLUserConsole);
+		} catch (Exception $e) {
 			IssueLog::Error('Core:GetConsoleUsersQuota:Error : '.$e->getMessage());
 			throw new Exception(Dict::Format('Core:GetQuota:Error', Dict::S('Core:ConsoleUsers')));
 		}
@@ -65,16 +68,53 @@ class ITopUserQuotaRepository
 		return array_diff($aConsoleUsers, $aPortalUsers, $aReadOnlyUsers);
 	}

+	private function ArrayToOQLStringParameter(array $aValues): string
+	{
+		$aQuotedValues = [];
+		foreach ($aValues as $value) {
+			$value = trim((string) $value);
+			if ($value === '') {
+				continue;
+			}
+			$aQuotedValues[] = "'".addslashes($value)."'";
+		}
+
+		return empty($aQuotedValues) ? "''" : implode(', ', $aQuotedValues);
+	}
+
+	/**
+	 * @throws CoreUnexpectedValue
+	 * @throws CoreException
+	 * @throws MySQLException
+	 */
+	public function GetUsersFromFilter(DBObjectSearch|DBUnionSearch|null $oFilter, array $aOrderBy = [], array $aArgs = []): array
+	{
+		$aUsers = [];
+		if (is_null($oFilter)) {
+			return $aUsers;
+		}
+		$oSet = new DBObjectSet($oFilter, $aOrderBy, $aArgs);
+		while ($oUser = $oSet->Fetch()) {
+			$aUsers[] = $oUser;
+		}
+
+		return $aUsers;
+	}
+
 	/**
 	 * @throws Exception
 	 */
-	public function GetApplicationUsers(bool $bAllData = true): array
+	public function GetPortalUsers(bool $bAllData = true): array
 	{
-		$sOQLApplicationUser = 'SELECT UserToken';
+		$sOQLPortalUser = "
+        SELECT User AS u
+        JOIN URP_UserProfile AS uup ON uup.userid = u.id
+        JOIN URP_Profiles AS up ON uup.profileid = up.id
+		WHERE up.id = '2' AND u.status != 'disabled' ";
+
 		try {
-			$oFilter = $bAllData ? DBObjectSearch::FromOQL_AllData($sOQLApplicationUser) : DBObjectSearch::FromOQL($sOQLApplicationUser);
-		}
-		catch (Exception $e) {
+			$oFilter = $bAllData ? DBObjectSearch::FromOQL_AllData($sOQLPortalUser) : DBObjectSearch::FromOQL($sOQLPortalUser);
+		} catch (Exception $e) {
 			IssueLog::Error('Core:GetConsoleUsersQuota:Error : '.$e->getMessage());
 			throw new Exception(Dict::Format('Core:GetQuota:Error', Dict::S('Core:ApplicationUsers')));
 		}
@@ -82,72 +122,6 @@ class ITopUserQuotaRepository
 		return $this->GetUsersFromFilter($oFilter);
 	}

-	/**
-	 * @throws Exception
-	 */
-	public function GetDisabledUsers(bool $bAllData = true): array
-	{
-		$sOQLDisabledUser = "
-		SELECT User AS u
-		WHERE u.status = 'disabled'
-		";
-		try {
-			$oFilter = $bAllData ? DBObjectSearch::FromOQL_AllData($sOQLDisabledUser) : DBObjectSearch::FromOQL($sOQLDisabledUser);
-		}
-		catch (Exception $e) {
-			IssueLog::Error('Core:GetDisabledUsersQuota:Error : '.$e->getMessage());
-			throw new Exception(Dict::Format('Core:GetQuota:Error', Dict::S('Core:DisabledUsers')));
-		}
-
-		return $this->GetUsersFromFilter($oFilter);
-	}
-
-	/**
-	 * @throws CoreException
-	 * @throws MySQLException
-	 * @throws CoreUnexpectedValue
-	 * @throws OQLException
-	 * @throws ArchivedObjectException
-	 * @throws DictExceptionUnknownLanguage
-	 */
-	private function IsUserReadOnly(User $oUser, string $sClassCategory): bool
-{
-	if ($oUser->Get('status') == 'disabled') {
-		return false;
-	}
-
-	// check if user is a portal user
-	$oProfileLinks = $oUser->Get('profile_list');
-	while ($oLink = $oProfileLinks->Fetch()) {
-		$iProfileId = $oLink->Get('profileid');
-		if (!$iProfileId) {
-			continue;
-		}
-		$oProfile = MetaModel::GetObject('URP_Profiles', $iProfileId, false);
-		if ($oProfile && $oProfile->Get('name') === PORTAL_PROFILE_NAME) {
-			return false;
-		}
-	}
-
-	// login (mandatory to compute rights)
-	UserRights::Login($oUser->GetName());
-
-	foreach (MetaModel::GetClasses($sClassCategory) as $sClass) {
-		// no need to check stimulis for now since users can't execute stimulus without UR_ACTION_MODIFY
-		if (
-			UserRights::IsActionAllowed($sClass, UR_ACTION_MODIFY, null, $oUser) ||
-			UserRights::IsActionAllowed($sClass, UR_ACTION_BULK_MODIFY, null, $oUser) ||
-			UserRights::IsActionAllowed($sClass, UR_ACTION_DELETE, null, $oUser) ||
-			UserRights::IsActionAllowed($sClass, UR_ACTION_BULK_DELETE, null, $oUser)
-		 ) {
-			UserRights::Logoff();
-			return false;
-		}
-	}
-	UserRights::Logoff();
-	return true;
-}
-
 	/**
 	 * @throws DictExceptionMissingString
 	 * @throws CoreException
@@ -178,48 +152,6 @@ class ITopUserQuotaRepository
 		return array_diff($aReadOnlyUsers, $aDisabledUsers);
 	}

-	/**
-	 * @throws Exception
-	 */
-	public function GetPortalUsers(bool $bAllData = true): array
-	{
-		$sOQLPortalUser = '
-        SELECT User AS u
-        JOIN URP_UserProfile AS uup ON uup.userid = u.id
-        JOIN URP_Profiles AS up ON uup.profileid = up.id
-		WHERE up.name = \' '.PORTAL_PROFILE_NAME.'\'';
-
-		try {
-			$oFilter = $bAllData ? DBObjectSearch::FromOQL_AllData($sOQLPortalUser) : DBObjectSearch::FromOQL($sOQLPortalUser);
-		}
-		catch (Exception $e) {
-			IssueLog::Error('combodo-users-quota-slave/GetUsersInQuota : '.$e->getMessage(), 'combodo-users-quota');
-			throw new Exception(Dict::Format('Core:GetQuota:Error', Dict::S('Core:PortalUsers')));
-		}
-
-		// TODO remove read only users
-		return $this->GetUsersFromFilter($oFilter);
-	}
-
-	/**
-	 * @throws CoreUnexpectedValue
-	 * @throws CoreException
-	 * @throws MySQLException
-	 */
-	public function GetUsersFromFilter(DBObjectSearch|DBUnionSearch|null $oFilter, array $aOrderBy = [], array $aArgs = []): array
-	{
-		$aUsers = [];
-		if (is_null($oFilter)) {
-			return $aUsers;
-		}
-		$oSet = new DBObjectSet($oFilter, $aOrderBy, $aArgs);
-		while ($oUser = $oSet->Fetch()) {
-			$aUsers[] = $oUser;
-		}
-
-		return $aUsers;
-	}
-
 	/**
 	 * @throws Exception
 	 */
@@ -229,13 +161,92 @@ class ITopUserQuotaRepository

 		try {
 			$oFilter = $bAllData ? DBObjectSearch::FromOQL_AllData($sOqlUser) : DBObjectSearch::FromOQL($sOqlUser);
-		}
-		catch (Exception $e) {
+		} catch (Exception $e) {
 			IssueLog::Error('combodo-users-quota-slave/GetUsersNotInQuota : '.$e->getMessage(), 'combodo-users-quota');
 			throw new Exception(Dict::S('CombodoUserQuota:Error'));
 		}
-		
+
 		return $this->GetUsersFromFilter($oFilter);
 	}

-}
+	/**
+	 * @throws CoreException
+	 * @throws MySQLException
+	 * @throws CoreUnexpectedValue
+	 * @throws OQLException
+	 * @throws ArchivedObjectException
+	 * @throws DictExceptionUnknownLanguage
+	 */
+	private function IsUserReadOnly(User $oUser, string $sClassCategory): bool
+	{
+		if ($oUser->Get('status') == 'disabled') {
+			return false;
+		}
+
+		// check if user is a portal user
+		$oProfileLinks = $oUser->Get('profile_list');
+		while ($oLink = $oProfileLinks->Fetch()) {
+			$iProfileId = $oLink->Get('profileid');
+			if (!$iProfileId) {
+				continue;
+			}
+			$oProfile = MetaModel::GetObject('URP_Profiles', $iProfileId, false);
+			if ($oProfile && $oProfile->Get('name') === PORTAL_PROFILE_NAME) {
+				return false;
+			}
+		}
+
+		// login (mandatory to compute rights)
+		UserRights::Login($oUser->GetName());
+
+		foreach (MetaModel::GetClasses($sClassCategory) as $sClass) {
+			// no need to check stimuli for now since users can't execute stimulus without UR_ACTION_MODIFY
+			if (
+				UserRights::IsActionAllowed($sClass, UR_ACTION_MODIFY, null, $oUser) ||
+				UserRights::IsActionAllowed($sClass, UR_ACTION_BULK_MODIFY, null, $oUser) ||
+				UserRights::IsActionAllowed($sClass, UR_ACTION_DELETE, null, $oUser) ||
+				UserRights::IsActionAllowed($sClass, UR_ACTION_BULK_DELETE, null, $oUser)
+			) {
+				UserRights::Logoff();
+				return false;
+			}
+		}
+		UserRights::Logoff();
+		return true;
+	}
+
+	/**
+	 * @throws Exception
+	 */
+	public function GetDisabledUsers(bool $bAllData = true): array
+	{
+		$sOQLDisabledUser = "
+		SELECT User AS u
+		WHERE u.status = 'disabled'
+		";
+		try {
+			$oFilter = $bAllData ? DBObjectSearch::FromOQL_AllData($sOQLDisabledUser) : DBObjectSearch::FromOQL($sOQLDisabledUser);
+		} catch (Exception $e) {
+			IssueLog::Error('Core:GetDisabledUsersQuota:Error : '.$e->getMessage());
+			throw new Exception(Dict::Format('Core:GetQuota:Error', Dict::S('Core:DisabledUsers')));
+		}
+
+		return $this->GetUsersFromFilter($oFilter);
+	}
+
+	/**
+	 * @throws Exception
+	 */
+	public function GetApplicationUsers(bool $bAllData = true): array
+	{
+		$sOQLApplicationUser = 'SELECT UserToken';
+		try {
+			$oFilter = $bAllData ? DBObjectSearch::FromOQL_AllData($sOQLApplicationUser) : DBObjectSearch::FromOQL($sOQLApplicationUser);
+		} catch (Exception $e) {
+			IssueLog::Error('Core:GetConsoleUsersQuota:Error : '.$e->getMessage());
+			throw new Exception(Dict::Format('Core:GetQuota:Error', Dict::S('Core:ApplicationUsers')));
+		}
+
+		return $this->GetUsersFromFilter($oFilter);
+	}
+}
--- a/tests/php-unit-tests/unitary-tests/sources/Users/ITopUserCountingRepositoryTest.php
+++ b/tests/php-unit-tests/unitary-tests/sources/Users/ITopUserCountingRepositoryTest.php
@@ -3,10 +3,11 @@
 namespace Users;

 use Combodo\iTop\Test\UnitTest\ItopDataTestCase;
-use Combodo\iTop\Users\ITopUserQuotaRepository;
+use Combodo\iTop\Users\ITopUserCountingRepository;
 use User;

-class ITopUserQuotaRepositoryTest extends ItopDataTestCase{
+class ITopUserCountingRepositoryTest extends ItopDataTestCase
+{
 	protected function setUp(): void
 	{
 		parent::setUp();
@@ -24,7 +25,8 @@ class ITopUserQuotaRepositoryTest extends ItopDataTestCase{
 		$this->GivenUserInDB('qpf_z17H3232*"ré$"é', ['Configuration ReadOnly', 'Ticket ReadOnly', 'Service Catalog ReadOnly']);
 	}

-	private function CreateDisabledUser() {
+	private function CreateDisabledUser()
+	{
 		$sUser = $this->GivenUserInDB('qpf_z17H3232*"ré$"é', ['Configuration Manager']);
 		// get user by login
 		$oUser = \MetaModel::GetObjectByName('User', $sUser);
@@ -39,11 +41,11 @@ class ITopUserQuotaRepositoryTest extends ItopDataTestCase{
 	 * @throws \MySQLException
 	 * @throws \Exception
 	 */
-	public function testNotDuplicateInDifferentQuotas(): void
+	public function testNotDuplicateInDifferentCountsCategories(): void
 	{
-		$oITopUserRepository = new ITopUserQuotaRepository();
+		$oITopUserRepository = new ITopUserCountingRepository();

-		$aQuotaUsers = [
+		$aCountedUsers = [
 			'console' => $oITopUserRepository->GetConsoleUsers(),
 			'portal' => $oITopUserRepository->GetPortalUsers(),
 			'disabled' => $oITopUserRepository->GetDisabledUsers(),
@@ -51,31 +53,32 @@ class ITopUserQuotaRepositoryTest extends ItopDataTestCase{
 			'application' => $oITopUserRepository->GetApplicationUsers(),
 		];

-		$aUserToQuotas = [];
-		foreach ($aQuotaUsers as $sQuota => $aUsers) {
+		$aCountedUserFormated = [];
+		foreach ($aCountedUsers as $sCountedCategory => $aUsers) {
 			foreach ($aUsers as $oUser) {
 				$sUserId = (string) $oUser->GetKey();
-				$aUserToQuotas[$sUserId][$sQuota] = true;
+				$aCountedUserFormated[$sUserId][$sCountedCategory] = true;
 			}
 		}

 		$aDuplicates = [];
-		foreach ($aUserToQuotas as $sUserId => $aQuotas) {
-			$aQuotaNames = array_keys($aQuotas);
-			if (count($aQuotaNames) > 1) {
-				sort($aQuotaNames);
-				$aDuplicates[] = sprintf('User #%s appears in: %s', $sUserId, implode(', ', $aQuotaNames));
+		foreach ($aCountedUserFormated as $sUserId => $aCountedCategory) {
+			$aCountedCategoryName = array_keys($aCountedCategory);
+			if (count($aCountedCategoryName) > 1) {
+				sort($aCountedCategoryName);
+				$aDuplicates[] = sprintf('User #%s appears in: %s', $sUserId, implode(', ', $aCountedCategoryName));
 			}
 		}

 		$this->assertEmpty(
 			$aDuplicates,
-			"Some users are counted in multiple quotas:\n- ".implode("\n- ", $aDuplicates)
+			"Some users are counted in multiple categories:\n- ".implode("\n- ", $aDuplicates)
 		);
 	}

-	public function testAllUsersAreInQuota () {
-		$oITopUserRepository = new ITopUserQuotaRepository();
+	public function testAllUsersAreCounted()
+	{
+		$oITopUserRepository = new ITopUserCountingRepository();

 		$aConsoleUsers = $oITopUserRepository->GetConsoleUsers();
 		$aPortalUsers = $oITopUserRepository->GetPortalUsers();
@@ -83,16 +86,16 @@ class ITopUserQuotaRepositoryTest extends ItopDataTestCase{
 		$aReadOnlyUsers = $oITopUserRepository->GetReadOnlyUsers();
 		$aApplicationUsers = $oITopUserRepository->GetApplicationUsers();

-		$aAllUsersFromQuota = array_merge($aConsoleUsers, $aPortalUsers, $aDisabledUsers, $aReadOnlyUsers, $aApplicationUsers);
+		$aAllUsersFromMergedCounts = array_merge($aConsoleUsers, $aPortalUsers, $aDisabledUsers, $aReadOnlyUsers, $aApplicationUsers);

 		$aAllUsersFromOQL = $oITopUserRepository->GetAllUsers();

-		$this->assertEmpty(array_merge(array_diff($aAllUsersFromQuota, $aAllUsersFromOQL), array_diff($aAllUsersFromOQL, $aAllUsersFromQuota)));
+		$this->assertEmpty(array_merge(array_diff($aAllUsersFromMergedCounts, $aAllUsersFromOQL), array_diff($aAllUsersFromOQL, $aAllUsersFromMergedCounts)));
 	}

-	public function testAllUsersInQuotaAreUsersObjects ()
+	public function testAllCountedUsersAreUsersObjects()
 	{
-		$oITopUserRepository = new ITopUserQuotaRepository();
+		$oITopUserRepository = new ITopUserCountingRepository();

 		$aConsoleUsers = $oITopUserRepository->GetConsoleUsers();
 		$aPortalUsers = $oITopUserRepository->GetPortalUsers();
@@ -100,13 +103,12 @@ class ITopUserQuotaRepositoryTest extends ItopDataTestCase{
 		$aReadOnlyUsers = $oITopUserRepository->GetReadOnlyUsers();
 		$aApplicationUsers = $oITopUserRepository->GetApplicationUsers();

-		$aAllQuotaUsers = array_merge($aConsoleUsers, $aPortalUsers, $aDisabledUsers, $aReadOnlyUsers, $aApplicationUsers);
+		$aCountedUsers = array_merge($aConsoleUsers, $aPortalUsers, $aDisabledUsers, $aReadOnlyUsers, $aApplicationUsers);

-		foreach ($aAllQuotaUsers as $oUser) {
+		foreach ($aCountedUsers as $oUser) {
 			$this->assertInstanceOf(User::class, $oUser);
 		}
-		

 	}

-}
+}