diff --git a/core/config.class.inc.php b/core/config.class.inc.php index 6fb2d6ea8..ad1e210a0 100644 --- a/core/config.class.inc.php +++ b/core/config.class.inc.php @@ -1747,6 +1747,14 @@ class Config 'source_of_value' => '', 'show_in_conf_sample' => false, ], + 'developer_mode.interface_cache.enabled' => [ + 'type' => 'bool', + 'description' => 'If true then InterfaceDiscovery uses dynamic cache (in developer_mode)', + 'default' => false, + 'value' => false, + 'source_of_value' => '', + 'show_in_conf_sample' => false, + ], 'theme.enable_precompilation' => [ 'type' => 'bool', 'description' => 'If false, theme compilation will not use any precompiled file setup optimization.)', diff --git a/sources/Service/Cache/DataModelDependantCache.php b/sources/Service/Cache/DataModelDependantCache.php index 5a31de485..174903cef 100644 --- a/sources/Service/Cache/DataModelDependantCache.php +++ b/sources/Service/Cache/DataModelDependantCache.php @@ -23,16 +23,16 @@ class DataModelDependantCache { if (!isset(self::$oInstance)) { - self::$oInstance = new DataModelDependantCache(utils::GetCachePath()); + self::$oInstance = new DataModelDependantCache(); } return self::$oInstance; } private ?string $sStorageRootDir; // Nullable for test purposes - private function __construct($sStorageRootDir) + private function __construct() { - $this->sStorageRootDir = $sStorageRootDir; + $this->sStorageRootDir = null; } /** @@ -146,10 +146,25 @@ class DataModelDependantCache return $this->GetStorageRootDir()."/$sPool/"; } - /** Overridable for testing purposes */ + + /** + * for test purpose + * + * @return string + */ protected function GetStorageRootDir(): string { // Could be forced by tests return $this->sStorageRootDir ?? utils::GetCachePath(); } + + /** + * for test purpose + * + * @param string $sStorageRootDir + */ + public function SetStorageRootDir(?string $sStorageRootDir): void + { + $this->sStorageRootDir = $sStorageRootDir; + } } \ No newline at end of file diff --git a/sources/Service/InterfaceDiscovery/InterfaceDiscovery.php b/sources/Service/InterfaceDiscovery/InterfaceDiscovery.php index d786a69b9..22d13c031 100644 --- a/sources/Service/InterfaceDiscovery/InterfaceDiscovery.php +++ b/sources/Service/InterfaceDiscovery/InterfaceDiscovery.php @@ -6,24 +6,31 @@ use Combodo\iTop\Service\Cache\DataModelDependantCache; use Exception; use IssueLog; use LogChannels; +use MetaModel; use ReflectionClass; use utils; class InterfaceDiscovery { private static InterfaceDiscovery $oInstance; + private DataModelDependantCache $oCacheService; private ?array $aForcedClassMap = null; // For testing purposes + const CACHE_NONE = 'CACHE_NONE'; + const CACHE_DYNAMIC = 'CACHE_DYNAMIC'; + const CACHE_STATIC = 'CACHE_STATIC'; + private function __construct() { + $this->oCacheService = DataModelDependantCache::GetInstance(); } public static function GetInstance(): InterfaceDiscovery { - if (!isset(self::$oInstance)) - { + if (!isset(self::$oInstance)) { self::$oInstance = new InterfaceDiscovery(); } + return self::$oInstance; } @@ -36,6 +43,7 @@ class InterfaceDiscovery * @param array|null $aAdditionalExcludedPaths Optional list of paths to exclude from the search (partial names allowed, case sensitive, use / as separator) * * @return array of fully qualified class names + * @throws \ReflectionException when $sInterface is not an interface */ public function FindItopClasses(string $sInterface, ?array $aAdditionalExcludedPaths = null): array { @@ -44,14 +52,24 @@ class InterfaceDiscovery } $aExcludedPaths = array_merge(['/lib/', '/node_modules/', '/test/', '/tests/'], $aAdditionalExcludedPaths); + return $this->FindClasses($sInterface, $aExcludedPaths); } + /** + * @param string $sInterface + * @param array $aExcludedPaths + * + * @return array + * @throws \ReflectionException + */ private function FindClasses(string $sInterface, array $aExcludedPaths = []): array { - $sCacheUniqueKey = $this->MakeCacheKey($sInterface, $aExcludedPaths); - if ($this->IsCacheValid($sCacheUniqueKey)) { - return $this->ReadClassesFromCache($sCacheUniqueKey); + if ($this->GetCacheMode() !== self::CACHE_NONE) { + $sCacheUniqueKey = $this->MakeCacheKey($sInterface, $aExcludedPaths); + if ($this->IsCacheValid($sCacheUniqueKey)) { + return $this->ReadClassesFromCache($sCacheUniqueKey); + } } $sExcludedPathsRegExp = $this->GetExcludedPathsRegExp($aExcludedPaths); @@ -66,40 +84,65 @@ class InterfaceDiscovery } } - $this->SaveClassesToCache($sCacheUniqueKey, $aMatchingClasses, ['Interface' => $sInterface, 'Excluded paths' => implode(',', $aExcludedPaths)]); + if ($this->GetCacheMode() !== self::CACHE_NONE) { + $this->SaveClassesToCache($sCacheUniqueKey, $aMatchingClasses, ['Interface' => $sInterface, 'Excluded paths' => implode(',', $aExcludedPaths)]); + } return $aMatchingClasses; } + private function GetAutoloadClassMaps(): array { - $oCacheService = DataModelDependantCache::GetInstance(); - - $aAutoloadClassMaps = $oCacheService->Fetch('InterfaceDiscovery', 'autoload_classmaps'); - if ($aAutoloadClassMaps !== null) { - return $aAutoloadClassMaps; + if ($this->GetCacheMode() === self::CACHE_DYNAMIC) { + $aAutoloadClassMaps = $this->oCacheService->Fetch('InterfaceDiscovery', 'autoload_classmaps'); + if ($aAutoloadClassMaps !== null) { + return $aAutoloadClassMaps; + } } // guess all the autoload class maps from the extensions $aAutoloadClassMaps = glob(APPROOT.'env-'.utils::GetCurrentEnvironment().'/*/vendor/composer/autoload_classmap.php'); $aAutoloadClassMaps[] = APPROOT.'lib/composer/autoload_classmap.php'; - $oCacheService->Store('InterfaceDiscovery', 'autoload_classmaps', $aAutoloadClassMaps); + if ($this->GetCacheMode() === self::CACHE_DYNAMIC) { + $this->oCacheService->Store('InterfaceDiscovery', 'autoload_classmaps', $aAutoloadClassMaps); + } + return $aAutoloadClassMaps; } + /** + * @param string $sPHPClass + * @param string $sInterface + * + * @return bool + * + * @throws \ReflectionException + */ private function IsInterfaceImplementation(string $sPHPClass, string $sInterface): bool { try { $oRefClass = new ReflectionClass($sPHPClass); - } catch (Exception $e) { + } + catch (Exception $e) { return false; } - if (!$oRefClass->implementsInterface($sInterface)) return false; - if ($oRefClass->isInterface()) return false; - if ($oRefClass->isAbstract()) return false; - if ($oRefClass->isTrait()) return false; - if ($oRefClass->getName() !== $sPHPClass) return false; // Skip aliases + if (!$oRefClass->implementsInterface($sInterface)) { + return false; + } + if ($oRefClass->isInterface()) { + return false; + } + if ($oRefClass->isAbstract()) { + return false; + } + if ($oRefClass->isTrait()) { + return false; + } + if ($oRefClass->getName() !== $sPHPClass) { + return false; + } // Skip aliases return true; } @@ -110,8 +153,7 @@ class InterfaceDiscovery $aAutoloaderErrors = []; if ($this->aForcedClassMap !== null) { $aClassMap = $this->aForcedClassMap; - } - else { + } else { foreach ($this->GetAutoloadClassMaps() as $sAutoloadFile) { if (false === utils::RealPath($sAutoloadFile, APPROOT)) { // can happen when we still have the autoloader symlink in env-*, but it points to a file that no longer exists @@ -140,12 +182,18 @@ class InterfaceDiscovery private function IsValidPHPFile(string $sOptionalPHPFile, ?string $sExcludedPathsRegExp): bool { - if ($sOptionalPHPFile === '') return true; + if ($sOptionalPHPFile === '') { + return true; + } $sOptionalPHPFile = utils::LocalPath($sOptionalPHPFile); - if ($sOptionalPHPFile === false) return false; + if ($sOptionalPHPFile === false) { + return false; + } - if (is_null($sExcludedPathsRegExp)) return true; + if (is_null($sExcludedPathsRegExp)) { + return true; + } if (preg_match($sExcludedPathsRegExp, '/'.$sOptionalPHPFile) === 1) { return false; @@ -156,42 +204,51 @@ class InterfaceDiscovery private function IsCacheValid(string $sKey): bool { - if ($this->aForcedClassMap !== null) return false; + if ($this->aForcedClassMap !== null) { + return false; + } - $oCacheService = DataModelDependantCache::GetInstance(); + if (!$this->oCacheService->HasEntry('InterfaceDiscovery', $sKey)) { + return false; + } - if (!$oCacheService->HasEntry('InterfaceDiscovery', $sKey)) return false; - - if (!utils::IsDevelopmentEnvironment()) return true; + if ($this->GetCacheMode() === self::CACHE_STATIC) { + return true; + } // On development environment, we check the cache validity by comparing the cache file with the autoload_classmap files - $iCacheTime = $oCacheService->GetEntryModificationTime('InterfaceDiscovery', $sKey); - foreach($this->GetAutoloadClassMaps() as $sSourceFile) { + $iCacheTime = $this->oCacheService->GetEntryModificationTime('InterfaceDiscovery', $sKey); + foreach ($this->GetAutoloadClassMaps() as $sSourceFile) { $iSourceTime = filemtime($sSourceFile); - if ($iSourceTime > $iCacheTime) return false; + if ($iSourceTime > $iCacheTime) { + return false; + } } + return true; } public function ReadClassesFromCache(string $sKey): array { - $oCacheService = DataModelDependantCache::GetInstance(); - return $oCacheService->Fetch('InterfaceDiscovery', $sKey); + return $this->oCacheService->Fetch('InterfaceDiscovery', $sKey); } protected function SaveClassesToCache(string $sKey, array $aMatchingClasses, array $aMoreInfo): void { - if ($this->aForcedClassMap !== null) return; + if ($this->aForcedClassMap !== null) { + return; + } - $oCacheService = DataModelDependantCache::GetInstance(); - $oCacheService->Store('InterfaceDiscovery', $sKey, $aMatchingClasses, $aMoreInfo); + $this->oCacheService->Store('InterfaceDiscovery', $sKey, $aMatchingClasses, $aMoreInfo); } - private function GetExcludedPathsRegExp(array $aExcludedPaths) : ?string + private function GetExcludedPathsRegExp(array $aExcludedPaths): ?string { - if (count($aExcludedPaths) == 0) return null; + if (count($aExcludedPaths) == 0) { + return null; + } - $aExcludedPathRegExps = array_map(function($sPath) { + $aExcludedPathRegExps = array_map(function ($sPath) { return preg_quote($sPath, '#'); }, $aExcludedPaths); @@ -200,7 +257,36 @@ class InterfaceDiscovery protected function MakeCacheKey(string $sInterface, array $aExcludedPaths): string { - if (count($aExcludedPaths) == 0) return $sInterface; - return md5($sInterface.':'.implode(',', $aExcludedPaths)); + if (count($aExcludedPaths) == 0) { + $sKey = $sInterface; + } else { + $sKey = $sInterface.':'.implode(',', $aExcludedPaths); + } + + $iPos = strrpos($sInterface, '\\'); + $sInterfaceDisplayName = $iPos === false ? $sInterface : substr($sInterface, $iPos + 1); + + return md5($sKey)."-$sInterfaceDisplayName"; + } + + /** + * @param \Combodo\iTop\Service\Cache\DataModelDependantCache $this->oCacheService + */ + public function SetCacheService(DataModelDependantCache $oCacheService): void + { + $this->oCacheService = $oCacheService; + } + + protected function GetCacheMode(): string + { + if (!utils::IsDevelopmentEnvironment()) { + return self::CACHE_STATIC; + } + + if (MetaModel::GetConfig()->Get('developer_mode.interface_cache.enabled')) { + return self::CACHE_DYNAMIC; + } + + return self::CACHE_NONE; } } \ No newline at end of file diff --git a/tests/php-unit-tests/src/BaseTestCase/ItopTestCase.php b/tests/php-unit-tests/src/BaseTestCase/ItopTestCase.php index eb3f0e9bb..30366a4b5 100644 --- a/tests/php-unit-tests/src/BaseTestCase/ItopTestCase.php +++ b/tests/php-unit-tests/src/BaseTestCase/ItopTestCase.php @@ -507,20 +507,38 @@ abstract class ItopTestCase extends TestCase * * @since 3.2.0 */ - protected function AssertArraysHaveSameItems(array $aExpectedClasses, array $aClasses, string $sMessage = ''): void + protected function AssertArraysHaveSameItems(array $aExpected, array $aActual, string $sMessage = ''): void { - sort($aClasses); - sort($aExpectedClasses); + sort($aActual); + sort($aExpected); - $sExpected = implode("\n", $aExpectedClasses); - $sActual = implode("\n", $aClasses); + $sExpected = implode("\n", $aExpected); + $sActual = implode("\n", $aActual); if ($sExpected === $sActual) { $this->assertTrue(true); return; } $sMessage .= "\nExpected:\n$sExpected\nActual:\n$sActual"; - var_export($aClasses); + var_export($aActual); $this->fail($sMessage); } + + /** + * The order of the files is not important + * + * @since 3.2.1 + */ + public function AssertDirectoryListingEquals(array $aExpected, string $sDir, string $sMessage = ''): void + { + $aFiles = []; + + foreach (scandir($sDir) as $sFile) { + if ($sFile !== '.' && $sFile !== '..') { + $aFiles[] = basename($sFile); + } + } + + $this->AssertArraysHaveSameItems($aExpected, $aFiles, $sMessage); + } } diff --git a/tests/php-unit-tests/unitary-tests/sources/Service/Cache/DataModelDependantCacheTest.php b/tests/php-unit-tests/unitary-tests/sources/Service/Cache/DataModelDependantCacheTest.php index 4561816a0..f2db4c38a 100644 --- a/tests/php-unit-tests/unitary-tests/sources/Service/Cache/DataModelDependantCacheTest.php +++ b/tests/php-unit-tests/unitary-tests/sources/Service/Cache/DataModelDependantCacheTest.php @@ -4,7 +4,6 @@ namespace Combodo\iTop\Test\UnitTest\Service\Cache; use Combodo\iTop\Service\Cache\DataModelDependantCache; use Combodo\iTop\Test\UnitTest\ItopTestCase; -use Exception; class DataModelDependantCacheTest extends ItopTestCase { @@ -19,12 +18,12 @@ class DataModelDependantCacheTest extends ItopTestCase $this->sCacheRootDir = self::CreateTmpdir(); $this->oCacheService = DataModelDependantCache::GetInstance(); - $this->SetNonPublicProperty($this->oCacheService, 'sStorageRootDir', $this->sCacheRootDir); + $this->oCacheService->SetStorageRootDir($this->sCacheRootDir); } protected function tearDown(): void { - $this->SetNonPublicProperty($this->oCacheService, 'sStorageRootDir', null); + $this->oCacheService->SetStorageRootDir(null); self::RecurseRmdir($this->sCacheRootDir); parent::tearDown(); @@ -50,7 +49,7 @@ class DataModelDependantCacheTest extends ItopTestCase public function testShouldStoreInADirectoryRebuiltOnCompilation(): void { // Given the storage is reset to the default - $this->SetNonPublicProperty($this->oCacheService, 'sStorageRootDir', null); + $this->oCacheService->SetStorageRootDir(null); // Then $sFilePath = $this->InvokeNonPublicMethod(DataModelDependantCache::class, 'MakeCacheFileName', $this->oCacheService, ['pool-A', 'key']); diff --git a/tests/php-unit-tests/unitary-tests/sources/Service/InterfaceDiscovery/InterfaceDiscoveryTest.php b/tests/php-unit-tests/unitary-tests/sources/Service/InterfaceDiscovery/InterfaceDiscoveryTest.php index 912da490b..e3c8289a4 100644 --- a/tests/php-unit-tests/unitary-tests/sources/Service/InterfaceDiscovery/InterfaceDiscoveryTest.php +++ b/tests/php-unit-tests/unitary-tests/sources/Service/InterfaceDiscovery/InterfaceDiscoveryTest.php @@ -3,22 +3,38 @@ namespace Combodo\iTop\Test\UnitTest\Service\InterfaceDiscovery; use Combodo\iTop\Application\UI\Base\iUIBlockFactory; +use Combodo\iTop\Service\Cache\DataModelDependantCache; use Combodo\iTop\Service\InterfaceDiscovery\InterfaceDiscovery; use Combodo\iTop\Test\UnitTest\ItopDataTestCase; +use MetaModel; class InterfaceDiscoveryTest extends ItopDataTestCase { + private InterfaceDiscovery $oInterfaceDiscovery; + private string $sCacheRootDir; + private DataModelDependantCache $oCacheService; + + protected function setUp(): void + { + parent::setUp(); + $this->oInterfaceDiscovery = InterfaceDiscovery::GetInstance(); + $this->sCacheRootDir = self::CreateTmpdir(); + $this->oCacheService = DataModelDependantCache::GetInstance(); + $this->oCacheService->SetStorageRootDir($this->sCacheRootDir); + $this->oInterfaceDiscovery->SetCacheService($this->oCacheService); + } + protected function tearDown(): void { $this->SetNonPublicProperty(InterfaceDiscovery::GetInstance(), 'aForcedClassMap', null); + $this->oCacheService->SetStorageRootDir(null); + self::RecurseRmdir($this->sCacheRootDir); parent::tearDown(); } public function testShouldSelectTheRequestedItopClasses() { - $oInterfaceDiscoveryService = InterfaceDiscovery::GetInstance(); - - $this->GivenClassMap($oInterfaceDiscoveryService, [ + $this->GivenClassMap([ 'Combodo\iTop\Application\UI\Base\Component\Alert\Alert' => APPROOT . '/sources/Application/UI/Base/Component/Alert/Alert.php', 'Combodo\iTop\Application\UI\Base\Component\Alert\AlertUIBlockFactory' => APPROOT . '/sources/Application/UI/Base/Component/Alert/AlertUIBlockFactory.php', 'Combodo\iTop\Application\UI\Base\Component\ButtonGroup\ButtonGroupUIBlockFactory' => APPROOT . '/sources/Application/UI/Base/Component/ButtonGroup/ButtonGroupUIBlockFactory.php', @@ -29,29 +45,26 @@ class InterfaceDiscoveryTest extends ItopDataTestCase 'Combodo\iTop\Application\UI\Base\Component\Alert\AlertUIBlockFactory', 'Combodo\iTop\Application\UI\Base\Component\ButtonGroup\ButtonGroupUIBlockFactory', ], - $oInterfaceDiscoveryService->FindItopClasses(iUIBlockFactory::class) + $this->oInterfaceDiscovery->FindItopClasses(iUIBlockFactory::class) ); } public function testShouldExcludeSpecifiedDirectories() { - $oInterfaceDiscoveryService = InterfaceDiscovery::GetInstance(); - $this->GivenClassMap($oInterfaceDiscoveryService, [ + $this->GivenClassMap([ 'Combodo\iTop\Application\UI\Base\Component\Alert\AlertUIBlockFactory' => APPROOT . '/sources/Application/UI/Base/Component/Alert/AlertUIBlockFactory.php', 'Combodo\iTop\Application\UI\Base\Component\ButtonGroup\ButtonGroupUIBlockFactory' => APPROOT . '/sources/Application/UI/Base/Component/ButtonGroup/ButtonGroupUIBlockFactory.php', ]); $this->AssertArraysHaveSameItems( [], - $oInterfaceDiscoveryService->FindItopClasses(iUIBlockFactory::class, ['Component/ButtonGroup', '/Alert/']) + $this->oInterfaceDiscovery->FindItopClasses(iUIBlockFactory::class, ['Component/ButtonGroup', '/Alert/']) ); } public function testShouldExcludeAliases() { - $oInterfaceDiscoveryService = InterfaceDiscovery::GetInstance(); - - $this->GivenClassMap($oInterfaceDiscoveryService, [ + $this->GivenClassMap([ 'Combodo\iTop\Application\UI\Base\Component\Alert\AlertUIBlockFactory' => APPROOT . '/sources/Application/UI/Base/Component/Alert/AlertUIBlockFactory.php', 'AlbertIsBlockingTheFactory' => APPROOT . '/sources/Application/UI/Base/Component/Alert/AlertUIBlockFactory.php', ]); @@ -62,12 +75,48 @@ class InterfaceDiscoveryTest extends ItopDataTestCase [ 'Combodo\iTop\Application\UI\Base\Component\Alert\AlertUIBlockFactory', ], - $oInterfaceDiscoveryService->FindItopClasses(iUIBlockFactory::class) + $this->oInterfaceDiscovery->FindItopClasses(iUIBlockFactory::class) ); } - private function GivenClassMap(InterfaceDiscovery $oInterfaceDiscoveryService, array $aClassMap): void + public function testShouldNotProduceCacheForDevelopers() { - $this->SetNonPublicProperty($oInterfaceDiscoveryService, 'aForcedClassMap', $aClassMap); + DataModelDependantCache::GetInstance()->Clear('InterfaceDiscovery'); + + MetaModel::GetConfig()->Set('developer_mode.enabled', true); + MetaModel::GetConfig()->Set('developer_mode.interface_cache.enabled', false); + + $this->assertGreaterThan(0, count($this->oInterfaceDiscovery->FindItopClasses(iUIBlockFactory::class))); + $this->assertFileDoesNotExist($this->sCacheRootDir.'/InterfaceDiscovery'); + } + + public function testShouldProduceDynamicCacheForDevelopersWillingTo() + { + DataModelDependantCache::GetInstance()->Clear('InterfaceDiscovery'); + + MetaModel::GetConfig()->Set('developer_mode.enabled', true); + MetaModel::GetConfig()->Set('developer_mode.interface_cache.enabled', true); + + $this->assertGreaterThan(0, count($this->oInterfaceDiscovery->FindItopClasses(iUIBlockFactory::class))); + $this->AssertDirectoryListingEquals([ + 'autoload_classmaps.php', + '310db363d8e32bfcf57cbb3800912ea2_iUIBlockFactory.php' + ], + $this->sCacheRootDir.'/InterfaceDiscovery'); + } + + public function testShouldProduceStaticCacheForProduction() + { + DataModelDependantCache::GetInstance()->Clear('InterfaceDiscovery'); + + MetaModel::GetConfig()->Set('developer_mode.enabled', false); + + $this->assertGreaterThan(0, count($this->oInterfaceDiscovery->FindItopClasses(iUIBlockFactory::class))); + $this->AssertDirectoryListingEquals(['310db363d8e32bfcf57cbb3800912ea2_iUIBlockFactory.php'], $this->sCacheRootDir.'/InterfaceDiscovery'); + } + + private function GivenClassMap(array $aClassMap): void + { + $this->SetNonPublicProperty($this->oInterfaceDiscovery, 'aForcedClassMap', $aClassMap); } }