diff --git a/.phpstan-baseline.php b/.phpstan-baseline.php index d8b727a9bc6..139b8258a56 100644 --- a/.phpstan-baseline.php +++ b/.phpstan-baseline.php @@ -2473,12 +2473,6 @@ 'count' => 1, 'path' => __DIR__ . '/src/Glpi/Asset/Asset.php', ]; -$ignoreErrors[] = [ - 'message' => '#^Instanceof between Glpi\\\\Asset\\\\AssetDefinition and Glpi\\\\Asset\\\\AssetDefinition will always evaluate to true\\.$#', - 'identifier' => 'instanceof.alwaysTrue', - 'count' => 1, - 'path' => __DIR__ . '/src/Glpi/Asset/Asset.php', -]; $ignoreErrors[] = [ 'message' => '#^Method Glpi\\\\Asset\\\\Asset\\:\\:getById\\(\\) should return static\\(Glpi\\\\Asset\\\\Asset\\)\\|false but returns object\\.$#', 'identifier' => 'return.type', @@ -2503,24 +2497,12 @@ 'count' => 2, 'path' => __DIR__ . '/src/Glpi/Asset/AssetDefinitionManager.php', ]; -$ignoreErrors[] = [ - 'message' => '#^Instanceof between Glpi\\\\Asset\\\\AssetDefinition and Glpi\\\\Asset\\\\AssetDefinition will always evaluate to true\\.$#', - 'identifier' => 'instanceof.alwaysTrue', - 'count' => 1, - 'path' => __DIR__ . '/src/Glpi/Asset/AssetModel.php', -]; $ignoreErrors[] = [ 'message' => '#^Method Glpi\\\\Asset\\\\AssetModel\\:\\:getById\\(\\) should return static\\(Glpi\\\\Asset\\\\AssetModel\\)\\|false but returns Glpi\\\\Asset\\\\AssetModel\\.$#', 'identifier' => 'return.type', 'count' => 1, 'path' => __DIR__ . '/src/Glpi/Asset/AssetModel.php', ]; -$ignoreErrors[] = [ - 'message' => '#^Instanceof between Glpi\\\\Asset\\\\AssetDefinition and Glpi\\\\Asset\\\\AssetDefinition will always evaluate to true\\.$#', - 'identifier' => 'instanceof.alwaysTrue', - 'count' => 1, - 'path' => __DIR__ . '/src/Glpi/Asset/AssetType.php', -]; $ignoreErrors[] = [ 'message' => '#^Method Glpi\\\\Asset\\\\AssetType\\:\\:getById\\(\\) should return static\\(Glpi\\\\Asset\\\\AssetType\\)\\|false but returns Glpi\\\\Asset\\\\AssetType\\.$#', 'identifier' => 'return.type', @@ -2545,30 +2527,6 @@ 'count' => 1, 'path' => __DIR__ . '/src/Glpi/Asset/Capacity/CapacityInterface.php', ]; -$ignoreErrors[] = [ - 'message' => '#^Instanceof between Glpi\\\\Asset\\\\AssetDefinition and Glpi\\\\Asset\\\\AssetDefinition will always evaluate to true\\.$#', - 'identifier' => 'instanceof.alwaysTrue', - 'count' => 1, - 'path' => __DIR__ . '/src/Glpi/Asset/RuleDictionaryModel.php', -]; -$ignoreErrors[] = [ - 'message' => '#^Instanceof between Glpi\\\\Asset\\\\AssetDefinition and Glpi\\\\Asset\\\\AssetDefinition will always evaluate to true\\.$#', - 'identifier' => 'instanceof.alwaysTrue', - 'count' => 1, - 'path' => __DIR__ . '/src/Glpi/Asset/RuleDictionaryModelCollection.php', -]; -$ignoreErrors[] = [ - 'message' => '#^Instanceof between Glpi\\\\Asset\\\\AssetDefinition and Glpi\\\\Asset\\\\AssetDefinition will always evaluate to true\\.$#', - 'identifier' => 'instanceof.alwaysTrue', - 'count' => 1, - 'path' => __DIR__ . '/src/Glpi/Asset/RuleDictionaryType.php', -]; -$ignoreErrors[] = [ - 'message' => '#^Instanceof between Glpi\\\\Asset\\\\AssetDefinition and Glpi\\\\Asset\\\\AssetDefinition will always evaluate to true\\.$#', - 'identifier' => 'instanceof.alwaysTrue', - 'count' => 1, - 'path' => __DIR__ . '/src/Glpi/Asset/RuleDictionaryTypeCollection.php', -]; $ignoreErrors[] = [ 'message' => '#^Call to function is_string\\(\\) with string will always evaluate to true\\.$#', 'identifier' => 'function.alreadyNarrowedType', @@ -2953,12 +2911,6 @@ 'count' => 1, 'path' => __DIR__ . '/src/Glpi/Debug/ProfilerSection.php', ]; -$ignoreErrors[] = [ - 'message' => '#^Instanceof between Glpi\\\\Dropdown\\\\DropdownDefinition and Glpi\\\\Dropdown\\\\DropdownDefinition will always evaluate to true\\.$#', - 'identifier' => 'instanceof.alwaysTrue', - 'count' => 1, - 'path' => __DIR__ . '/src/Glpi/Dropdown/Dropdown.php', -]; $ignoreErrors[] = [ 'message' => '#^Method Glpi\\\\Dropdown\\\\Dropdown\\:\\:getById\\(\\) should return static\\(Glpi\\\\Dropdown\\\\Dropdown\\)\\|false but returns object\\.$#', 'identifier' => 'return.type', diff --git a/phpunit/DbTestCase.php b/phpunit/DbTestCase.php index d8c596b7ed7..fd6ccebae75 100644 --- a/phpunit/DbTestCase.php +++ b/phpunit/DbTestCase.php @@ -397,20 +397,6 @@ protected function initAssetDefinition( $this->callPrivateMethod($definition, 'getDecodedProfilesField') ); - // Clear definition cache - $rc = new ReflectionClass(\Glpi\CustomObject\AbstractDefinitionManager::class); - $rc->getProperty('definitions_data')->setValue(\Glpi\Asset\AssetDefinitionManager::getInstance(), []); - - $manager = \Glpi\Asset\AssetDefinitionManager::getInstance(); - $this->callPrivateMethod($manager, 'loadConcreteClass', $definition); - $this->callPrivateMethod($manager, 'loadConcreteModelClass', $definition); - $this->callPrivateMethod($manager, 'loadConcreteTypeClass', $definition); - $this->callPrivateMethod($manager, 'loadConcreteModelDictionaryCollectionClass', $definition); - $this->callPrivateMethod($manager, 'loadConcreteModelDictionaryClass', $definition); - $this->callPrivateMethod($manager, 'loadConcreteTypeDictionaryCollectionClass', $definition); - $this->callPrivateMethod($manager, 'loadConcreteTypeDictionaryClass', $definition); - $this->callPrivateMethod($manager, 'boostrapConcreteClass', $definition); - return $definition; } @@ -472,14 +458,6 @@ protected function enableCapacity( $this->callPrivateMethod($definition, 'getDecodedCapacitiesField') ); - // Force boostrap to trigger methods such as "onClassBootstrap" - $manager = AssetDefinitionManager::getInstance(); - $this->callPrivateMethod( - $manager, - 'boostrapConcreteClass', - $definition - ); - return $definition; } @@ -520,14 +498,6 @@ protected function disableCapacity( $this->callPrivateMethod($definition, 'getDecodedCapacitiesField') ); - // Force boostrap to trigger methods such as "onClassBootstrap" - $manager = AssetDefinitionManager::getInstance(); - $this->callPrivateMethod( - $manager, - 'boostrapConcreteClass', - $definition - ); - return $definition; } } diff --git a/phpunit/GLPITestCase.php b/phpunit/GLPITestCase.php index ce2ebe38f4e..de34f1cc91c 100644 --- a/phpunit/GLPITestCase.php +++ b/phpunit/GLPITestCase.php @@ -34,6 +34,7 @@ */ use Glpi\Asset\AssetDefinitionManager; +use Glpi\Dropdown\DropdownDefinitionManager; use Glpi\Tests\Log\TestHandler; use Monolog\Level; use Monolog\Logger; @@ -87,6 +88,9 @@ public function setUp(): void $SQLLOGGER->setHandlers([$this->sql_log_handler]); vfsStreamWrapper::register(); + + AssetDefinitionManager::getInstance()->registerAutoload(); + DropdownDefinitionManager::getInstance()->registerAutoload(); } public function tearDown(): void diff --git a/src/Glpi/Asset/Asset.php b/src/Glpi/Asset/Asset.php index ab3686872dc..00eca4f03f4 100644 --- a/src/Glpi/Asset/Asset.php +++ b/src/Glpi/Asset/Asset.php @@ -62,13 +62,13 @@ abstract class Asset extends CommonDBTM use \Glpi\Features\Inventoriable; /** - * Asset definition. + * Asset definition system name. * * Must be defined here to make PHPStan happy (see https://github.com/phpstan/phpstan/issues/8808). * Must be defined by child class too to ensure that assigning a value to this property will affect * each child classe independently. */ - protected static AssetDefinition $definition; + protected static string $definition_system_name; final public function __construct() { @@ -84,11 +84,12 @@ final public function __construct() */ public static function getDefinition(): AssetDefinition { - if (!(static::$definition instanceof AssetDefinition)) { + $definition = AssetDefinitionManager::getInstance()->getDefinition(static::$definition_system_name); + if (!($definition instanceof AssetDefinition)) { throw new \RuntimeException('Asset definition is expected to be defined in concrete class.'); } - return static::$definition; + return $definition; } public static function getDefinitionClass(): string diff --git a/src/Glpi/Asset/AssetDefinition.php b/src/Glpi/Asset/AssetDefinition.php index 19475e325f3..b4932dc4a16 100644 --- a/src/Glpi/Asset/AssetDefinition.php +++ b/src/Glpi/Asset/AssetDefinition.php @@ -8,7 +8,6 @@ * http://glpi-project.org * * @copyright 2015-2024 Teclib' and contributors. - * @copyright 2003-2014 by the INDEPNET Development Team. * @licence https://www.gnu.org/licenses/gpl-3.0.html * * --------------------------------------------------------------------- @@ -306,6 +305,14 @@ protected function prepareInput(array $input): array|bool public function post_addItem() { + parent::post_addItem(); + + // Trigger the `onCapacityEnabled` hooks. + $added_capacities = @json_decode($this->fields['capacities']); + foreach ($added_capacities as $capacity_classname) { + $this->onCapacityEnabled($capacity_classname); + } + // Add default display preferences for the new asset definition $prefs = [ 4, // Name @@ -324,14 +331,13 @@ public function post_addItem() 'users_id' => 0, ]); } - - parent::post_addItem(); } public function post_updateItem($history = true) { + parent::post_updateItem(); + if (in_array('capacities', $this->updates)) { - // When capabilities are removed, trigger the cleaning of data related to this capacity. $new_capacities = @json_decode($this->fields['capacities']); $old_capacities = @json_decode($this->oldvalues['capacities']); @@ -352,28 +358,25 @@ public function post_updateItem($history = true) return; } - $removed_capacities = array_diff($old_capacities, $new_capacities); - $rights_to_remove = []; - foreach ($removed_capacities as $capacity_classname) { - $capacity = AssetDefinitionManager::getInstance()->getCapacity($capacity_classname); - if ($capacity === null) { - // can be null if provided by a plugin that is no longer active - continue; - } - $capacity->onCapacityDisabled($this->getAssetClassName()); - array_push($rights_to_remove, ...$capacity->getSpecificRights()); + $added_capacities = array_diff($new_capacities, $old_capacities); + foreach ($added_capacities as $capacity_classname) { + $this->onCapacityEnabled($capacity_classname); } - if (count($rights_to_remove) > 0) { - $this->cleanRights($rights_to_remove); + $removed_capacities = array_diff($old_capacities, $new_capacities); + foreach ($removed_capacities as $capacity_classname) { + $this->onCapacityDisabled($capacity_classname); } } - - parent::post_updateItem(); } public function cleanDBonPurge() { + $capacities = $this->getDecodedCapacitiesField(); + foreach ($capacities as $capacity_classname) { + $this->onCapacityDisabled($capacity_classname); + } + $related_classes = [ $this->getAssetClassName(), $this->getAssetModelClassName(), @@ -389,6 +392,41 @@ public function cleanDBonPurge() } } + /** + * Handle the activation of a capacity. + * + * @phpstan-param class-string<\Glpi\Asset\Capacity\CapacityInterface> $capacity_classname + */ + private function onCapacityEnabled(string $capacity_classname): void + { + $capacity = AssetDefinitionManager::getInstance()->getCapacity($capacity_classname); + if ($capacity === null) { + // can be null if provided by a plugin that is no longer active + return; + } + $capacity->onCapacityEnabled($this->getAssetClassName()); + } + + /** + * Handle the deactivation of a capacity. + * + * @phpstan-param class-string<\Glpi\Asset\Capacity\CapacityInterface> $capacity_classname + */ + private function onCapacityDisabled(string $capacity_classname): void + { + $capacity = AssetDefinitionManager::getInstance()->getCapacity($capacity_classname); + if ($capacity === null) { + // can be null if provided by a plugin that is no longer active + return; + } + $capacity->onCapacityDisabled($this->getAssetClassName()); + + $rights_to_remove = $capacity->getSpecificRights(); + if (count($rights_to_remove) > 0) { + $this->cleanRights($rights_to_remove); + } + } + public function rawSearchOptions() { $search_options = parent::rawSearchOptions(); diff --git a/src/Glpi/Asset/AssetDefinitionManager.php b/src/Glpi/Asset/AssetDefinitionManager.php index b436272d9b4..c8f52c79a4d 100644 --- a/src/Glpi/Asset/AssetDefinitionManager.php +++ b/src/Glpi/Asset/AssetDefinitionManager.php @@ -178,7 +178,7 @@ public function getReservedSystemNames(): array return $names; } - protected function boostrapConcreteClass(AbstractDefinition $definition): void + public function bootstrapDefinition(AbstractDefinition $definition): void { /** @var array $CFG_GLPI */ global $CFG_GLPI; @@ -197,12 +197,18 @@ protected function boostrapConcreteClass(AbstractDefinition $definition): void 'unicity_types' ]; foreach ($config_keys as $config_key) { - $CFG_GLPI[$config_key][] = $asset_class_name; + if (!in_array($asset_class_name, $CFG_GLPI[$config_key], true)) { + $CFG_GLPI[$config_key][] = $asset_class_name; + } } // Add type and model to dictionnary config entry - $CFG_GLPI['dictionnary_types'][] = $definition->getAssetTypeClassName(); - $CFG_GLPI['dictionnary_types'][] = $definition->getAssetModelClassName(); + if (!in_array($definition->getAssetTypeClassName(), $CFG_GLPI['dictionnary_types'], true)) { + $CFG_GLPI['dictionnary_types'][] = $definition->getAssetTypeClassName(); + } + if (!in_array($definition->getAssetModelClassName(), $CFG_GLPI['dictionnary_types'], true)) { + $CFG_GLPI['dictionnary_types'][] = $definition->getAssetModelClassName(); + } // Bootstrap capacities foreach ($capacities as $capacity) { @@ -378,20 +384,13 @@ private function loadConcreteClass(AssetDefinition $definition): void namespace Glpi\\CustomAsset; use Glpi\\Asset\\Asset; -use Glpi\\Asset\\AssetDefinition; final class {$definition->getAssetClassName(false)} extends Asset { - protected static AssetDefinition \$definition; + protected static string \$definition_system_name = '{$definition->fields['system_name']}'; public static \$rightname = '{$rightname}'; } PHP ); - - // Set the definition of the concrete class using reflection API. - // It permits to directly store a pointer to the definition on the object without having - // to make the property publicly writable. - $reflected_class = new ReflectionClass($definition->getAssetClassName()); - $reflected_class->setStaticPropertyValue('definition', $definition); } /** @@ -407,16 +406,12 @@ private function loadConcreteModelClass(AssetDefinition $definition): void namespace Glpi\\CustomAsset; use Glpi\\Asset\\AssetModel; -use Glpi\\Asset\\AssetDefinition; final class {$definition->getAssetModelClassName(false)} extends AssetModel { - protected static AssetDefinition \$definition; + protected static string \$definition_system_name = '{$definition->fields['system_name']}'; } PHP ); - - $reflected_class = new ReflectionClass($definition->getAssetModelClassName()); - $reflected_class->setStaticPropertyValue('definition', $definition); } /** @@ -432,16 +427,12 @@ private function loadConcreteTypeClass(AssetDefinition $definition): void namespace Glpi\\CustomAsset; use Glpi\\Asset\\AssetType; -use Glpi\\Asset\\AssetDefinition; final class {$definition->getAssetTypeClassName(false)} extends AssetType { - protected static AssetDefinition \$definition; + protected static string \$definition_system_name = '{$definition->fields['system_name']}'; } PHP ); - - $reflected_class = new ReflectionClass($definition->getAssetTypeClassName()); - $reflected_class->setStaticPropertyValue('definition', $definition); } private function loadConcreteModelDictionaryClass(AssetDefinition $definition): void @@ -449,18 +440,14 @@ private function loadConcreteModelDictionaryClass(AssetDefinition $definition): eval(<<getAssetModelDictionaryClassName(false)} extends RuleDictionaryModel { - protected static AssetDefinition \$definition; + protected static string \$definition_system_name = '{$definition->fields['system_name']}'; } PHP ); - - $reflected_class = new ReflectionClass($definition->getAssetModelDictionaryClassName()); - $reflected_class->setStaticPropertyValue('definition', $definition); } private function loadConcreteTypeDictionaryClass(AssetDefinition $definition): void @@ -468,18 +455,14 @@ private function loadConcreteTypeDictionaryClass(AssetDefinition $definition): v eval(<<getAssetTypeDictionaryClassName(false)} extends RuleDictionaryType { - protected static AssetDefinition \$definition; + protected static string \$definition_system_name = '{$definition->fields['system_name']}'; } PHP ); - - $reflected_class = new ReflectionClass($definition->getAssetTypeDictionaryClassName()); - $reflected_class->setStaticPropertyValue('definition', $definition); } private function loadConcreteModelDictionaryCollectionClass(AssetDefinition $definition): void @@ -487,18 +470,14 @@ private function loadConcreteModelDictionaryCollectionClass(AssetDefinition $def eval(<<getAssetModelDictionaryCollectionClassName(false)} extends RuleDictionaryModelCollection { - protected static AssetDefinition \$definition; + protected static string \$definition_system_name = '{$definition->fields['system_name']}'; } PHP ); - - $reflected_class = new ReflectionClass($definition->getAssetModelDictionaryCollectionClassName()); - $reflected_class->setStaticPropertyValue('definition', $definition); } private function loadConcreteTypeDictionaryCollectionClass(AssetDefinition $definition): void @@ -506,17 +485,13 @@ private function loadConcreteTypeDictionaryCollectionClass(AssetDefinition $defi eval(<<getAssetTypeDictionaryCollectionClassName(false)} extends RuleDictionaryTypeCollection { - protected static AssetDefinition \$definition; + protected static string \$definition_system_name = '{$definition->fields['system_name']}'; } PHP ); - - $reflected_class = new ReflectionClass($definition->getAssetTypeDictionaryCollectionClassName()); - $reflected_class->setStaticPropertyValue('definition', $definition); } } diff --git a/src/Glpi/Asset/AssetModel.php b/src/Glpi/Asset/AssetModel.php index 66b1b84596b..cb811aff985 100644 --- a/src/Glpi/Asset/AssetModel.php +++ b/src/Glpi/Asset/AssetModel.php @@ -42,13 +42,13 @@ abstract class AssetModel extends \CommonDCModelDropdown { /** - * Asset definition. + * Asset definition system name. * * Must be defined here to make PHPStan happy (see https://github.com/phpstan/phpstan/issues/8808). * Must be defined by child class too to ensure that assigning a value to this property will affect * each child classe independently. */ - protected static AssetDefinition $definition; + protected static string $definition_system_name; /** * Get the asset definition related to concrete class. @@ -57,12 +57,14 @@ abstract class AssetModel extends \CommonDCModelDropdown */ public static function getDefinition(): AssetDefinition { - if (!(static::$definition instanceof AssetDefinition)) { + $definition = AssetDefinitionManager::getInstance()->getDefinition(static::$definition_system_name); + if (!($definition instanceof AssetDefinition)) { throw new \RuntimeException('Asset definition is expected to be defined in concrete class.'); } - return static::$definition; + return $definition; } + public static function getTypeName($nb = 0) { return sprintf(_n('%s model', '%s models', $nb), static::getDefinition()->getTranslatedName()); diff --git a/src/Glpi/Asset/AssetType.php b/src/Glpi/Asset/AssetType.php index e64842983a7..be8db7f6234 100644 --- a/src/Glpi/Asset/AssetType.php +++ b/src/Glpi/Asset/AssetType.php @@ -41,13 +41,13 @@ abstract class AssetType extends CommonType { /** - * Asset definition. + * Asset definition system name. * * Must be defined here to make PHPStan happy (see https://github.com/phpstan/phpstan/issues/8808). * Must be defined by child class too to ensure that assigning a value to this property will affect * each child classe independently. */ - protected static AssetDefinition $definition; + protected static string $definition_system_name; /** * Get the asset definition related to concrete class. @@ -56,11 +56,12 @@ abstract class AssetType extends CommonType */ public static function getDefinition(): AssetDefinition { - if (!(static::$definition instanceof AssetDefinition)) { + $definition = AssetDefinitionManager::getInstance()->getDefinition(static::$definition_system_name); + if (!($definition instanceof AssetDefinition)) { throw new \RuntimeException('Asset definition is expected to be defined in concrete class.'); } - return static::$definition; + return $definition; } public static function getTypeName($nb = 0) diff --git a/src/Glpi/Asset/Capacity/AbstractCapacity.php b/src/Glpi/Asset/Capacity/AbstractCapacity.php index d845d67bef1..0eb628bed5c 100644 --- a/src/Glpi/Asset/Capacity/AbstractCapacity.php +++ b/src/Glpi/Asset/Capacity/AbstractCapacity.php @@ -186,6 +186,10 @@ public function onObjectInstanciation(Asset $object): void { } + public function onCapacityEnabled(string $classname): void + { + } + public function onCapacityDisabled(string $classname): void { } diff --git a/src/Glpi/Asset/Capacity/CapacityInterface.php b/src/Glpi/Asset/Capacity/CapacityInterface.php index 19e10ca82fb..3c74e92ffc5 100644 --- a/src/Glpi/Asset/Capacity/CapacityInterface.php +++ b/src/Glpi/Asset/Capacity/CapacityInterface.php @@ -109,6 +109,14 @@ public function getCapacityUsageDescription(string $classname): string; */ public function onClassBootstrap(string $classname): void; + /** + * Method executed when capacity is enabled on given asset class. + * + * @param class-string<\Glpi\Asset\Asset> $classname + * @return void + */ + public function onCapacityEnabled(string $classname): void; + /** * Method executed when capacity is disabled on given asset class. * diff --git a/src/Glpi/Asset/Capacity/IsInventoriableCapacity.php b/src/Glpi/Asset/Capacity/IsInventoriableCapacity.php index a5df4caa224..dfbdf529fca 100644 --- a/src/Glpi/Asset/Capacity/IsInventoriableCapacity.php +++ b/src/Glpi/Asset/Capacity/IsInventoriableCapacity.php @@ -100,7 +100,10 @@ public function onClassBootstrap(string $classname): void CommonGLPI::registerStandardTab($classname, Item_Environment::class, 85); CommonGLPI::registerStandardTab($classname, Item_Process::class, 85); + } + public function onCapacityEnabled(string $classname): void + { //create rules $rules = new \RuleImportAsset(); $rules->initRules(true, $classname); diff --git a/src/Glpi/Asset/RuleDictionaryModel.php b/src/Glpi/Asset/RuleDictionaryModel.php index 8c9858e2b80..d4e7e8ccc93 100644 --- a/src/Glpi/Asset/RuleDictionaryModel.php +++ b/src/Glpi/Asset/RuleDictionaryModel.php @@ -41,13 +41,13 @@ abstract class RuleDictionaryModel extends RuleDictionnaryDropdown { /** - * Asset definition. + * Asset definition system name. * * Must be defined here to make PHPStan happy (see https://github.com/phpstan/phpstan/issues/8808). * Must be defined by child class too to ensure that assigning a value to this property will affect * each child classe independently. */ - protected static AssetDefinition $definition; + protected static string $definition_system_name; /** * Get the asset definition related to concrete class. @@ -56,11 +56,12 @@ abstract class RuleDictionaryModel extends RuleDictionnaryDropdown */ public static function getDefinition(): AssetDefinition { - if (!(static::$definition instanceof AssetDefinition)) { + $definition = AssetDefinitionManager::getInstance()->getDefinition(static::$definition_system_name); + if (!($definition instanceof AssetDefinition)) { throw new \RuntimeException('Asset definition is expected to be defined in concrete class.'); } - return static::$definition; + return $definition; } public function getCriterias() diff --git a/src/Glpi/Asset/RuleDictionaryModelCollection.php b/src/Glpi/Asset/RuleDictionaryModelCollection.php index 3952932a942..d3fe0b72921 100644 --- a/src/Glpi/Asset/RuleDictionaryModelCollection.php +++ b/src/Glpi/Asset/RuleDictionaryModelCollection.php @@ -39,13 +39,13 @@ abstract class RuleDictionaryModelCollection extends RuleDictionnaryDropdownCollection { /** - * Asset definition. + * Asset definition system name. * * Must be defined here to make PHPStan happy (see https://github.com/phpstan/phpstan/issues/8808). * Must be defined by child class too to ensure that assigning a value to this property will affect * each child classe independently. */ - protected static AssetDefinition $definition; + protected static string $definition_system_name; /** * Get the asset definition related to concrete class. @@ -54,11 +54,12 @@ abstract class RuleDictionaryModelCollection extends RuleDictionnaryDropdownColl */ public static function getDefinition(): AssetDefinition { - if (!(static::$definition instanceof AssetDefinition)) { + $definition = AssetDefinitionManager::getInstance()->getDefinition(static::$definition_system_name); + if (!($definition instanceof AssetDefinition)) { throw new \RuntimeException('Asset definition is expected to be defined in concrete class.'); } - return static::$definition; + return $definition; } public function __construct() diff --git a/src/Glpi/Asset/RuleDictionaryType.php b/src/Glpi/Asset/RuleDictionaryType.php index 0e0132f5527..bad1902bd93 100644 --- a/src/Glpi/Asset/RuleDictionaryType.php +++ b/src/Glpi/Asset/RuleDictionaryType.php @@ -40,13 +40,13 @@ abstract class RuleDictionaryType extends RuleDictionnaryDropdown { /** - * Asset definition. + * Asset definition system name. * * Must be defined here to make PHPStan happy (see https://github.com/phpstan/phpstan/issues/8808). * Must be defined by child class too to ensure that assigning a value to this property will affect * each child classe independently. */ - protected static AssetDefinition $definition; + protected static string $definition_system_name; /** * Get the asset definition related to concrete class. @@ -55,11 +55,12 @@ abstract class RuleDictionaryType extends RuleDictionnaryDropdown */ public static function getDefinition(): AssetDefinition { - if (!(static::$definition instanceof AssetDefinition)) { + $definition = AssetDefinitionManager::getInstance()->getDefinition(static::$definition_system_name); + if (!($definition instanceof AssetDefinition)) { throw new \RuntimeException('Asset definition is expected to be defined in concrete class.'); } - return static::$definition; + return $definition; } public function getCriterias() diff --git a/src/Glpi/Asset/RuleDictionaryTypeCollection.php b/src/Glpi/Asset/RuleDictionaryTypeCollection.php index 0f77560e25a..417c804122a 100644 --- a/src/Glpi/Asset/RuleDictionaryTypeCollection.php +++ b/src/Glpi/Asset/RuleDictionaryTypeCollection.php @@ -39,13 +39,13 @@ abstract class RuleDictionaryTypeCollection extends RuleDictionnaryDropdownCollection { /** - * Asset definition. + * Asset definition system name. * * Must be defined here to make PHPStan happy (see https://github.com/phpstan/phpstan/issues/8808). * Must be defined by child class too to ensure that assigning a value to this property will affect * each child classe independently. */ - protected static AssetDefinition $definition; + protected static string $definition_system_name; /** * Get the asset definition related to concrete class. @@ -54,11 +54,12 @@ abstract class RuleDictionaryTypeCollection extends RuleDictionnaryDropdownColle */ public static function getDefinition(): AssetDefinition { - if (!(static::$definition instanceof AssetDefinition)) { + $definition = AssetDefinitionManager::getInstance()->getDefinition(static::$definition_system_name); + if (!($definition instanceof AssetDefinition)) { throw new \RuntimeException('Asset definition is expected to be defined in concrete class.'); } - return static::$definition; + return $definition; } public function __construct() diff --git a/src/Glpi/Config/LegacyConfigurators/CustomObjectsBootstrap.php b/src/Glpi/Config/LegacyConfigurators/CustomObjectsBootstrap.php index 491d2887015..fda3f7144af 100644 --- a/src/Glpi/Config/LegacyConfigurators/CustomObjectsBootstrap.php +++ b/src/Glpi/Config/LegacyConfigurators/CustomObjectsBootstrap.php @@ -50,8 +50,8 @@ public function execute(): void } Profiler::getInstance()->start('CustomObjectsBootstrap::execute', Profiler::CATEGORY_BOOT); - AssetDefinitionManager::getInstance()->bootstrapClasses(); - DropdownDefinitionManager::getInstance()->bootstrapClasses(); + AssetDefinitionManager::getInstance()->bootstrapDefinitions(); + DropdownDefinitionManager::getInstance()->bootstrapDefinitions(); Profiler::getInstance()->stop('CustomObjectsBootstrap::execute'); } } diff --git a/src/Glpi/CustomObject/AbstractDefinition.php b/src/Glpi/CustomObject/AbstractDefinition.php index 50cf13bc8e4..dfda2e7ddee 100644 --- a/src/Glpi/CustomObject/AbstractDefinition.php +++ b/src/Glpi/CustomObject/AbstractDefinition.php @@ -490,6 +490,13 @@ protected function prepareInput(array $input): array|bool public function post_addItem() { + // Clear the definitions cache to ensure that the code triggerred by the capacities hooks + // will not use an outdated definition list. + static::getDefinitionManagerClass()::getInstance()->clearDefinitionsCache(); + + // Bootstrap the definition to make it usable right now. + static::getDefinitionManagerClass()::getInstance()->bootstrapDefinition($this); + if ($this->isActive()) { $this->syncProfilesRights(); unset($_SESSION['menu']); @@ -498,6 +505,13 @@ public function post_addItem() public function post_updateItem($history = true) { + // Clear the definitions cache to ensure that the code triggerred by the capacities hooks + // will not use an outdated definition list. + static::getDefinitionManagerClass()::getInstance()->clearDefinitionsCache(); + + // Bootstrap the definition to make it usable right now. + static::getDefinitionManagerClass()::getInstance()->bootstrapDefinition($this); + if (in_array('is_active', $this->updates, true)) { // Force menu refresh when active state change unset($_SESSION['menu']); diff --git a/src/Glpi/CustomObject/AbstractDefinitionManager.php b/src/Glpi/CustomObject/AbstractDefinitionManager.php index 79038a1fd79..d488709d8e1 100644 --- a/src/Glpi/CustomObject/AbstractDefinitionManager.php +++ b/src/Glpi/CustomObject/AbstractDefinitionManager.php @@ -78,26 +78,24 @@ final public function registerAutoload(): void */ abstract public function autoloadClass(string $classname): void; - final public function bootstrapClasses(): void + /** + * Boostrap all the active definitions. + */ + final public function bootstrapDefinitions(): void { - foreach ($this->getDefinitions() as $definition) { - if (!$definition->isActive()) { - continue; - } - - $this->boostrapConcreteClass($definition); + foreach ($this->getDefinitions(true) as $definition) { + $this->bootstrapDefinition($definition); } } /** - * Bootstrap the concrete class. + * Bootstrap the definition. * @param AbstractDefinition $definition * @phpstan-param ConcreteDefinition $definition * @return void */ - protected function boostrapConcreteClass(AbstractDefinition $definition): void + public function bootstrapDefinition(AbstractDefinition $definition) { - // Intentionally left blank } final public function getCustomObjectClassNames(bool $with_namespace = true): array @@ -115,18 +113,27 @@ final public function getCustomObjectClassNames(bool $with_namespace = true): ar } /** - * Get the dropdown definition corresponding to given system name. + * Get the definition corresponding to given system name. * * @param string $system_name * @phpstan-return ConcreteDefinition|null */ - final protected function getDefinition(string $system_name): ?AbstractDefinition + final public function getDefinition(string $system_name): ?AbstractDefinition { return $this->getDefinitions()[$system_name] ?? null; } /** - * Get all the dropdown definitions. + * Clear the definitions cache. + */ + final public function clearDefinitionsCache(): void + { + $definition_class = static::getDefinitionClass(); + unset($this->definitions_data[$definition_class]); + } + + /** + * Get all the definitions. * * @param bool $only_active * @return AbstractDefinition[] diff --git a/src/Glpi/Dropdown/Dropdown.php b/src/Glpi/Dropdown/Dropdown.php index cf11f49d434..5212bfb5864 100644 --- a/src/Glpi/Dropdown/Dropdown.php +++ b/src/Glpi/Dropdown/Dropdown.php @@ -43,13 +43,13 @@ abstract class Dropdown extends CommonTreeDropdown use CustomObjectTrait; /** - * Dropdown definition. + * Dropdown definition system name. * * Must be defined here to make PHPStan happy (see https://github.com/phpstan/phpstan/issues/8808). * Must be defined by child class too to ensure that assigning a value to this property will affect * each child classe independently. */ - protected static DropdownDefinition $definition; + protected static string $definition_system_name; /** * Get the dropdown definition related to concrete class. @@ -58,11 +58,12 @@ abstract class Dropdown extends CommonTreeDropdown */ public static function getDefinition(): DropdownDefinition { - if (!(static::$definition instanceof DropdownDefinition)) { + $definition = DropdownDefinitionManager::getInstance()->getDefinition(static::$definition_system_name); + if (!($definition instanceof DropdownDefinition)) { throw new \RuntimeException('Dropdown definition is expected to be defined in concrete class.'); } - return static::$definition; + return $definition; } /** diff --git a/src/Glpi/Dropdown/DropdownDefinition.php b/src/Glpi/Dropdown/DropdownDefinition.php index d8e94c6f9a4..2c0411afe4f 100644 --- a/src/Glpi/Dropdown/DropdownDefinition.php +++ b/src/Glpi/Dropdown/DropdownDefinition.php @@ -156,6 +156,8 @@ public function prepareInputForAdd($input) public function post_addItem() { + parent::post_addItem(); + // Add default display preferences for the new definition $prefs = [ 14, // Name @@ -168,8 +170,6 @@ public function post_addItem() 'users_id' => 0, ]); } - - parent::post_addItem(); } public function cleanDBonPurge() diff --git a/src/Glpi/Dropdown/DropdownDefinitionManager.php b/src/Glpi/Dropdown/DropdownDefinitionManager.php index 14610db59c2..66808bade43 100644 --- a/src/Glpi/Dropdown/DropdownDefinitionManager.php +++ b/src/Glpi/Dropdown/DropdownDefinitionManager.php @@ -120,19 +120,12 @@ private function loadConcreteClass(DropdownDefinition $definition): void namespace Glpi\\CustomDropdown; use Glpi\\Dropdown\\Dropdown; -use Glpi\\Dropdown\\DropdownDefinition; final class {$definition->getDropdownClassName(false)} extends Dropdown { - protected static DropdownDefinition \$definition; + protected static string \$definition_system_name = '{$definition->fields['system_name']}'; public static \$rightname = '{$rightname}'; } PHP ); - - // Set the definition of the concrete class using reflection API. - // It permits to directly store a pointer to the definition on the object without having - // to make the property publicly writable. - $reflected_class = new ReflectionClass($definition->getDropdownClassName()); - $reflected_class->setStaticPropertyValue('definition', $definition); } } diff --git a/tests/DbTestCase.php b/tests/DbTestCase.php index aeb169ddf01..efd0f37d285 100644 --- a/tests/DbTestCase.php +++ b/tests/DbTestCase.php @@ -354,20 +354,6 @@ protected function initAssetDefinition( $this->array($this->callPrivateMethod($definition, 'getDecodedCapacitiesField'))->isEqualTo($capacities); $this->array($this->callPrivateMethod($definition, 'getDecodedProfilesField'))->isEqualTo($profiles); - // Clear definition cache - $rc = new ReflectionClass(\Glpi\CustomObject\AbstractDefinitionManager::class); - $rc->getProperty('definitions_data')->setValue(\Glpi\Asset\AssetDefinitionManager::getInstance(), []); - - $manager = \Glpi\Asset\AssetDefinitionManager::getInstance(); - $this->callPrivateMethod($manager, 'loadConcreteClass', $definition); - $this->callPrivateMethod($manager, 'loadConcreteModelClass', $definition); - $this->callPrivateMethod($manager, 'loadConcreteTypeClass', $definition); - $this->callPrivateMethod($manager, 'loadConcreteModelDictionaryCollectionClass', $definition); - $this->callPrivateMethod($manager, 'loadConcreteModelDictionaryClass', $definition); - $this->callPrivateMethod($manager, 'loadConcreteTypeDictionaryCollectionClass', $definition); - $this->callPrivateMethod($manager, 'loadConcreteTypeDictionaryClass', $definition); - $this->callPrivateMethod($manager, 'boostrapConcreteClass', $definition); - return $definition; } @@ -402,14 +388,6 @@ protected function initDropdownDefinition( ); $this->array($this->callPrivateMethod($definition, 'getDecodedProfilesField'))->isEqualTo($profiles); - // Clear definition cache - $rc = new ReflectionClass(\Glpi\CustomObject\AbstractDefinitionManager::class); - $rc->getProperty('definitions_data')->setValue(\Glpi\Dropdown\DropdownDefinitionManager::getInstance(), []); - - $manager = \Glpi\Dropdown\DropdownDefinitionManager::getInstance(); - $this->callPrivateMethod($manager, 'loadConcreteClass', $definition); - $this->callPrivateMethod($manager, 'boostrapConcreteClass', $definition); - return $definition; } @@ -470,14 +448,6 @@ protected function enableCapacity( $this->callPrivateMethod($definition, 'getDecodedCapacitiesField') )->contains($capacity); - // Force boostrap to trigger methods such as "onClassBootstrap" - $manager = AssetDefinitionManager::getInstance(); - $this->callPrivateMethod( - $manager, - 'boostrapConcreteClass', - $definition - ); - return $definition; } @@ -517,14 +487,6 @@ protected function disableCapacity( $this->callPrivateMethod($definition, 'getDecodedCapacitiesField') )->notContains($capacity); - // Force boostrap to trigger methods such as "onClassBootstrap" - $manager = AssetDefinitionManager::getInstance(); - $this->callPrivateMethod( - $manager, - 'boostrapConcreteClass', - $definition - ); - return $definition; } } diff --git a/tests/functional/Glpi/Asset/AssetDefinition.php b/tests/functional/Glpi/Asset/AssetDefinition.php index fc9e08c9093..032412c0103 100644 --- a/tests/functional/Glpi/Asset/AssetDefinition.php +++ b/tests/functional/Glpi/Asset/AssetDefinition.php @@ -432,7 +432,6 @@ public function testDelete() { /** @var \Glpi\Asset\AssetDefinition $definition */ $definition = $this->initAssetDefinition('test'); - \Glpi\Asset\AssetDefinitionManager::getInstance()->bootstrapClasses(); $this->createItem( $definition->getAssetClassName(), diff --git a/tests/functional/Glpi/Asset/AssetDefinitionManager.php b/tests/functional/Glpi/Asset/AssetDefinitionManager.php index 3f225ea946f..20bce59259b 100644 --- a/tests/functional/Glpi/Asset/AssetDefinitionManager.php +++ b/tests/functional/Glpi/Asset/AssetDefinitionManager.php @@ -56,7 +56,7 @@ public function testLoadConcreteClass(): void foreach ($mapping as $expected_classname => $definition) { $this->boolean(class_exists($expected_classname))->isTrue(); - $this->object($expected_classname::getDefinition())->isEqualTo($definition); + $this->array($expected_classname::getDefinition()->fields)->isEqualTo($definition->fields); } } @@ -196,15 +196,6 @@ public function testCommonITILTabRegistration( Asset $asset, array $expected_tabs ): void { - // Force the boostrap process to be recomputed (it is only computed once - // per execution in normal circumstances) - $manager = \Glpi\Asset\AssetDefinitionManager::getInstance(); - $this->callPrivateMethod( - $manager, - 'boostrapConcreteClass', - $definition - ); - // Get all tabs $tabs = $asset->defineAllTabs(); diff --git a/tests/functional/Glpi/Dropdown/DropdownDefinition.php b/tests/functional/Glpi/Dropdown/DropdownDefinition.php index fa1ba0efdf2..7fef94a0c57 100644 --- a/tests/functional/Glpi/Dropdown/DropdownDefinition.php +++ b/tests/functional/Glpi/Dropdown/DropdownDefinition.php @@ -368,7 +368,6 @@ public function testDelete() { /** @var \Glpi\Dropdown\DropdownDefinition $definition */ $definition = $this->initDropdownDefinition('test'); - \Glpi\Dropdown\DropdownDefinitionManager::getInstance()->bootstrapClasses(); $this->createItem( $definition->getDropdownClassName(), diff --git a/tests/functional/Glpi/Dropdown/DropdownDefinitionManager.php b/tests/functional/Glpi/Dropdown/DropdownDefinitionManager.php index cc426abcfbc..c83dbc95ae8 100644 --- a/tests/functional/Glpi/Dropdown/DropdownDefinitionManager.php +++ b/tests/functional/Glpi/Dropdown/DropdownDefinitionManager.php @@ -37,7 +37,6 @@ use DbTestCase; use Glpi\Dropdown\Dropdown; -use Glpi\Dropdown\DropdownDefinition; class DropdownDefinitionManager extends DbTestCase { @@ -52,7 +51,7 @@ public function testLoadConcreteClass(): void foreach ($mapping as $expected_classname => $definition) { $this->boolean(class_exists($expected_classname))->isTrue(); - $this->object($expected_classname::getDefinition())->isEqualTo($definition); + $this->array($expected_classname::getDefinition()->fields)->isEqualTo($definition->fields); } }