N°2982 - Speed up SCSS themes compilation during setup : take cascaded included imports in theme precompilation + class/method documentation cleanup

application/themehandler.class.inc.php

@@ -190,31 +190,27 @@ class ThemeHandler
 
 		$aThemeParametersWithVersion = self::CloneThemeParameterAndIncludeVersion($aThemeParameters, $sSetupCompilationTimestampInSecunds);
 
-		$sTmpThemeScssContent = '';
-		$iStyleLastModified = 0;
 		clearstatcache();
-		// Loading files to import and stylesheet to compile, also getting most recent modification time on overall files
 
-		$aStylesheetFiles = [];
+		// Loading files to import and stylesheet to compile, also getting most recent modification time on overall files
+		$sTmpThemeScssContent = '';
+		$oFindStylesheetObject = new FindStylesheetObject();
 		foreach ($aThemeParameters['imports'] as $sImport)
 		{
-			$sTmpThemeScssContent .= '@import "'.$sImport.'";'."\n";
-
-			$sFile = static::FindStylesheetFile($sImport, $aImportsPaths);
-			$iImportLastModified = @filemtime($sFile);
-			$aStylesheetFiles[] = $sFile;
-			$iStyleLastModified = $iStyleLastModified < $iImportLastModified ? $iImportLastModified : $iStyleLastModified;
+			static::FindStylesheetFile($sImport, $aImportsPaths, $oFindStylesheetObject);
 		}
 		foreach ($aThemeParameters['stylesheets'] as $sStylesheet)
 		{
-			$sTmpThemeScssContent .= '@import "'.$sStylesheet.'";'."\n";
-
-			$sFile = static::FindStylesheetFile($sStylesheet, $aImportsPaths);
-			$iStylesheetLastModified = @filemtime($sFile);
-			$aStylesheetFiles[] = $sFile;
-			$iStyleLastModified = $iStyleLastModified < $iStylesheetLastModified ? $iStylesheetLastModified : $iStyleLastModified;
+			static::FindStylesheetFile($sStylesheet, $aImportsPaths, $oFindStylesheetObject);
 		}
 
+		$aStylesheetFiles = $oFindStylesheetObject->GetAllStylesheetFiles();
+		foreach ($aStylesheetFiles as $sStylesheet){
+			$sTmpThemeScssContent .= '@import "'.$sStylesheet.'";'."\n";
+		}
+
+		$iStyleLastModified = $oFindStylesheetObject->GetLastModified();
+
 		$aIncludedImages=static::GetIncludedImages($aThemeParametersWithVersion, $aStylesheetFiles, $sThemeId);
 		foreach ($aIncludedImages as $sImage)
 		{
@@ -284,6 +280,7 @@ CSS;
 	}
 
 	/**
+	 * @since 3.0.0 N°2982
 	 * Compute the signature of a theme defined by its theme parameters. The signature is a JSON structure of
 	 * 1) one MD5 of all the variables/values (JSON encoded)
 	 * 2) the MD5 of each stylesheet file
@@ -306,16 +303,33 @@ CSS;
 			'images' => []
 		];
 
+		$oFindStylesheetObject = new FindStylesheetObject();
+
 		foreach ($aThemeParameters['imports'] as $key => $sImport)
 		{
-			$sFile = static::FindStylesheetFile($sImport, $aImportsPaths);
-			$aSignature['stylesheets'][$key] = md5_file($sFile);
+			static::FindStylesheetFile($sImport, $aImportsPaths, $oFindStylesheetObject);
+			$sFile = $oFindStylesheetObject->GetLastStylesheetFile();
+			if (!empty($sFile)){
+				$aSignature['stylesheets'][$key] = md5_file($sFile);
+			}
 		}
 		foreach ($aThemeParameters['stylesheets'] as $key => $sStylesheet)
 		{
-			$sFile = static::FindStylesheetFile($sStylesheet, $aImportsPaths);
-			$aSignature['stylesheets'][$key] = md5_file($sFile);
+			static::FindStylesheetFile($sStylesheet, $aImportsPaths, $oFindStylesheetObject);
+			$sFile = $oFindStylesheetObject->GetLastStylesheetFile();
+
+			if (!empty($sFile)){
+				$aSignature['stylesheets'][$key] = md5_file($sFile);
+			}
 		}
+
+		$aFiles = $oFindStylesheetObject->GetAllImports();
+		if (count($aFiles) !== 0) {
+			foreach ($aFiles as $sFile) {
+				$aSignature['imports'][$sFile] = md5_file($sFile);
+			}
+		}
+
 		foreach ($aIncludedImages as $sImage)
 		{
 			if (is_file($sImage)) {
@@ -335,7 +349,7 @@ CSS;
 	 * @param string $sThemeId : used only for logging purpose
 	 *
 	 * @return array complete path of the images, but with slashes as dir separator instead of DIRECTORY_SEPARATOR
-	 * @since 3.0.0
+	 * @since 3.0.0 N°2982
 	 */
 	public static function GetIncludedImages($aThemeParametersVariables, $aStylesheetFiles, $sThemeId)
 	{
@@ -430,6 +444,7 @@ CSS;
 	}
 
 	/**
+	 * @since 3.0.0 N°2982
 	 * Complete url using provided variables. Example with $var=1: XX + $var => XX1
 	 * @param $aMap
 	 * @param $aThemeParametersVariables
@@ -462,6 +477,7 @@ CSS;
 	}
 
 	/**
+	 * @since 3.0.0 N°2982
 	 * Find missing variable values from SCSS content based on their name.
 	 *
 	 * @param $aThemeParametersVariables
@@ -520,6 +536,7 @@ CSS;
 	}
 
 	/**
+	 * @since 3.0.0 N°2982
 	 * @param $aFoundVariables
 	 * @param array $aToCompleteUrls
 	 * @param array $aCompleteUrls
@@ -564,6 +581,7 @@ CSS;
 	}
 
 	/**
+	 * @since 3.0.0 N°2982
 	 * Find all referenced URLs from a SCSS file.
 	 * @param $aThemeParametersVariables
 	 * @param $sStylesheetFile
@@ -622,6 +640,7 @@ CSS;
 	}
 
 	/**
+	 * @since 3.0.0 N°2982
 	 * Calculate url based on its template + variables.
 	 * @param $sUrlTemplate
 	 * @param $aFoundVariables
@@ -672,6 +691,7 @@ CSS;
 
 
 	/**
+	 * @since 3.0.0 N°2982
 	 * Extract the signature for a generated CSS file. The signature MUST be alone one line immediately
 	 * followed (on the next line) by the === SIGNATURE END === pattern
 	 *
@@ -700,6 +720,12 @@ CSS;
 		return $sPreviousLine;
 	}
 
+	/**
+	 * @since 3.0.0 N°2982
+	 * @param $JsonSignature
+	 *
+	 * @return false|mixed
+	 */
 	public static function GetVarSignature($JsonSignature)
 	{
 		$aJsonArray = json_decode($JsonSignature, true);
@@ -711,31 +737,69 @@ CSS;
 	}
 
 	/**
-	 * Find the given file in the list of ImportsPaths directory
+	 * @since 3.0.0 N°2982
+	 * Find the given file in the list '$aImportsPaths' of directory and all included stylesheets as well
+	 * Compute latest timestamp found among all found stylesheets
+	 *
 	 * @param string $sFile
 	 * @param string[] $aImportsPaths
-	 * @throws Exception
-	 * @return string
+	 * @param FindStylesheetObject $oFindStylesheetObject
+	 * @param bool $bImports
+	 *
+	 * @throws \Exception
 	 */
-	public static function FindStylesheetFile($sFile, $aImportsPaths)
+	public static function FindStylesheetFile(string $sFile, array $aImportsPaths, $oFindStylesheetObject, $bImports = false)
 	{
+		if (! $bImports) {
+			$oFindStylesheetObject->ResetLastStyleSheet();
+		}
+
 		foreach($aImportsPaths as $sPath)
 		{
-			$sImportedFile = realpath($sPath.'/'.$sFile);
+			$sFilePath = $sPath.'/'.$sFile;
+			$sImportedFile = realpath($sFilePath);
+			if ($sImportedFile === false){
+				// Handle shortcut syntax like @import "typo ;
+				// file matched: _typo.scss
+				$sShortCut = substr($sFilePath, strrpos($sFilePath, '/') + 1);
+				$sFilePath = str_replace($sShortCut, "_$sShortCut.scss", $sFilePath);
+				$sImportedFile = realpath($sFilePath);
+			}
+
 			if (file_exists($sImportedFile))
 			{
-				return $sImportedFile;
+				if ($bImports){
+					$oFindStylesheetObject->AddImport($sImportedFile);
+				}else{
+					$oFindStylesheetObject->AddStylesheet($sImportedFile);
+				}
+				$oFindStylesheetObject->UpdateLastModified($sImportedFile);
+
+				//Regexp matching on all included scss files : @import 'XXX.scss';
+				preg_match_all('/@import \s*[\"\']([^\"\']*)\s*[\"\']\s*;/', file_get_contents($sImportedFile), $aMatches);
+				if ( (is_array($aMatches)) && (count($aMatches)!==0) ){
+					foreach ($aMatches[1] as $sImportedFile){
+						if (! $oFindStylesheetObject->AlreadyFetched($sImportedFile)) {
+							self::FindStylesheetFile($sImportedFile, [ dirname($sFilePath) ], $oFindStylesheetObject, true);
+						}
+					}
+				}
 			}
 		}
-		return ''; // Not found, fail silently, maybe the SCSS compiler knowns better...
 	}
 
+	/**
+	 * @since 3.0.0 N°2982
+	 * Used for testing purpose
+	 * @param $oCompileCSSServiceMock
+	 */
 	public static function MockCompileCSSService($oCompileCSSServiceMock)
 	{
 		static::$oCompileCSSService = $oCompileCSSServiceMock;
 	}
 
 	/**
+	 * @since 3.0.0 N°2982
 	 * Clone variable array and include $version with bSetupCompilationTimestamp value
 	 * @param $aThemeParameters
 	 * @param $bSetupCompilationTimestamp
@@ -758,6 +822,86 @@ CSS;
 	}
 }
 
+/**
+ * @since 3.0.0 N°2982
+ * Class FindStylesheetObject: dedicated class to store computations made in method FindStylesheetFile.
+ */
+class FindStylesheetObject{
+
+	private $aStylesheetImports;
+	private $aAllStylesheetFiles;
+	private $sLastStyleSheet;
+	private $iLastModified;
+
+	/**
+	 * FindStylesheetObject constructor.
+	 */
+	public function __construct()
+	{
+		$this->aAllStylesheetFiles = [];
+		$this->aStylesheetImports = [];
+		$this->sLastStyleSheet = "";
+		$this->iLastModified = 0;
+	}
+
+	public function GetLastStylesheetFile(): string
+	{
+		return $this->sLastStyleSheet;
+	}
+
+	public function GetAllImports(): array
+	{
+		return $this->aStylesheetImports;
+	}
+
+	public function GetAllStylesheetFiles(): array
+	{
+		return $this->aAllStylesheetFiles;
+	}
+
+	public function GetLastModified() : int
+	{
+		return $this->iLastModified;
+	}
+
+	public function GetStylesheetImports(): array
+	{
+		return $this->aStylesheetImports;
+	}
+
+	public function GetLastStyleSheet(): string
+	{
+		return $this->sLastStyleSheet;
+	}
+
+	public function AddStylesheet(string $sStylesheetFile): void
+	{
+		$this->aAllStylesheetFiles[] = $sStylesheetFile;
+		$this->sLastStyleSheet = $sStylesheetFile;
+	}
+
+	public function AlreadyFetched(string $sStylesheetFile) : bool {
+		return in_array($sStylesheetFile, $this->aAllStylesheetFiles)
+			|| in_array($sStylesheetFile, $this->aStylesheetImports);
+	}
+
+	public function AddImport(string $sStylesheetFile): void
+	{
+		$this->aAllStylesheetFiles[] = $sStylesheetFile;
+		$this->aStylesheetImports[] = $sStylesheetFile;
+	}
+
+	public function UpdateLastModified(string $sStylesheetFile): void
+	{
+		$this->iLastModified = max($this->iLastModified, @filemtime($sStylesheetFile));
+	}
+
+	public function ResetLastStyleSheet(): void
+	{
+		$this->sLastStyleSheet = "";
+	}
+}
+
 class CompileCSSService
 {
 	/**

setup/compiler.class.inc.php

@@ -2902,6 +2902,7 @@ EOF;
 	}
 
 	/**
+	 * @since 3.0.0 N°2982
 	 * Choose between precompiled files declared in datamodel XMLs or latest precompiled files generated after latest setup.
 	 *
 	 * @param string $sTempTargetDir

test/application/ThemeHandlerTest.php

@@ -6,7 +6,7 @@ use Combodo\iTop\Test\UnitTest\ItopTestCase;
  * @runTestsInSeparateProcesses
  * @preserveGlobalState disabled
  * @backupGlobals disabled
- * @covers utils
+ * @covers ThemeHandler
  */
 class ThemeHandlerTest extends ItopTestCase
 {
@@ -159,15 +159,15 @@ class ThemeHandlerTest extends ItopTestCase
 							$aThemeParameters['variables'][$sVariableId] = $oVariable->GetText();
 						}
 
-						$aStylesheetFiles = [];
 						/** @var \DOMNodeList $oImports */
 						$oImports = $oTheme->GetNodes('imports/import');
+						$oFindStylesheetObject = new FindStylesheetObject();
+
 						foreach ($oImports as $oImport)
 						{
 							$sImportId = $oImport->getAttribute('id');
 							$aThemeParameters['imports'][$sImportId] = $oImport->GetText();
-							$sFile = ThemeHandler::FindStylesheetFile($oImport->GetText(), $aImportsPaths);
-							$aStylesheetFiles[] = $sFile;
+							ThemeHandler::FindStylesheetFile($oImport->GetText(), $aImportsPaths, $oFindStylesheetObject);
 						}
 
 						/** @var \DOMNodeList $oStylesheets */
@@ -176,11 +176,10 @@ class ThemeHandlerTest extends ItopTestCase
 						{
 							$sStylesheetId = $oStylesheet->getAttribute('id');
 							$aThemeParameters['stylesheets'][$sStylesheetId] = $oStylesheet->GetText();
-							$sFile = ThemeHandler::FindStylesheetFile($oStylesheet->GetText(), $aImportsPaths);
-							$aStylesheetFiles[] = $sFile;
+							ThemeHandler::FindStylesheetFile($oImport->GetText(), $aImportsPaths, $oFindStylesheetObject);
 						}
 
-						$aIncludedImages = ThemeHandler::GetIncludedImages($aThemeParameters['variables'], $aStylesheetFiles, $sThemeId);
+						$aIncludedImages = ThemeHandler::GetIncludedImages($aThemeParameters['variables'], $oFindStylesheetObject->GetAllStylesheetFiles(), $sThemeId);
 						$compiled_json_sig = ThemeHandler::ComputeSignature($aThemeParameters, $aImportsPaths, $aIncludedImages);
 						//echo "  current signature: $compiled_json_sig\n";
 
@@ -603,6 +602,59 @@ SCSS;
 		$this->assertEquals($aExpectedImages, $aIncludedImages);
 	}
 
+	/**
+	 * @dataProvider FindStylesheetFileProvider
+	 * @throws \Exception
+	 */
+	public function testFindStylesheetFile(string $sFileToFind, array $aAllStylesheetFiles, array $aAllImports){
+		$aImportsPath = $this->sTmpDir.'/branding/';
+
+		$oFindStylesheetObject = new FindStylesheetObject();
+		ThemeHandler::FindStylesheetFile($sFileToFind, [$aImportsPath], $oFindStylesheetObject);
+
+		$this->assertEquals($aAllStylesheetFiles, str_replace($aImportsPath, "", $oFindStylesheetObject->GetAllStylesheetFiles()));
+		$this->assertEquals($aAllImports, str_replace($aImportsPath, "", $oFindStylesheetObject->GetAllImports()));
+		$this->assertEquals($aImportsPath . $aAllStylesheetFiles[0], $oFindStylesheetObject->GetLastStyleSheet());
+	}
+
+	public function FindStylesheetFileProvider(){
+		$sFileToFind3 = "css/multi_imports.scss";
+		$sFileToFind4 = "css/included_file1.scss";
+		$sFileToFind5 = "css/included_scss/included_file2.scss";
+
+		return [
+			"single file to find" => [
+				"sFileToFind" => "css/DO_NOT_CHANGE.light-grey.scss",
+				"aAllStylesheetFiles" => ["css/DO_NOT_CHANGE.light-grey.scss"],
+				"aAllImports" => []
+			],
+			"scss with simple @imports" => [
+				"sFileToFind" => "css/simple_import.scss",
+				"aAllStylesheetFiles" => ["css/simple_import.scss", $sFileToFind4],
+				"aAllImports" => [$sFileToFind4]
+			],
+			"scss with simple @imports in another folder" => [
+				"sFileToFind" => "css/simple_import2.scss",
+				"aAllStylesheetFiles" => ["css/simple_import2.scss", $sFileToFind5],
+				"aAllImports" => [$sFileToFind5]
+			],
+			"scss with @imports shortcut typography => _typography.scss" => [
+				"sFileToFind" => "css/short_cut.scss",
+				"aAllStylesheetFiles" => ["css/short_cut.scss", "css/_included_file3.scss"],
+				"aAllImports" => ["css/_included_file3.scss"]
+			],
+			"scss with multi @imports" => [
+				"sFileToFind" => $sFileToFind3,
+				"aAllStylesheetFiles" => [
+					$sFileToFind3,
+					$sFileToFind4,
+					$sFileToFind5
+				],
+				"aAllImports" => [$sFileToFind4, $sFileToFind5]
+			]
+		];
+	}
+
 	/**
 	 * @param $sPath
 	 * @param $sExpectedCanonicalPath

test/application/theme-handler/expected/css/multi_imports.scss

@@ -0,0 +1,8 @@
+
+
+@import 'included_file1.scss';
+@import 'included_scss/included_file2.scss';
+
+/*!
+ * Combodo portal template v1.0.0
+*/

test/application/theme-handler/expected/css/short_cut.scss

@@ -0,0 +1,2 @@
+
+@import 'included_file3';

test/application/theme-handler/expected/css/simple_import.scss

@@ -0,0 +1,2 @@
+
+@import 'included_file1.scss';

test/application/theme-handler/expected/css/simple_import2.scss

@@ -0,0 +1,2 @@
+
+@import 'included_scss/included_file2.scss';