From c8dd19c22fcc69aa983cd5986d4734f2b855d080 Mon Sep 17 00:00:00 2001 From: odain Date: Fri, 12 Feb 2021 09:45:22 +0100 Subject: [PATCH] =?UTF-8?q?N=C2=B02982=20-=20Speed=20up=20SCSS=20themes=20?= =?UTF-8?q?compilation=20during=20setup=20:=20take=20cascaded=20included?= =?UTF-8?q?=20imports=20in=20theme=20precompilation=20+=20class/method=20d?= =?UTF-8?q?ocumentation=20cleanup?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- application/themehandler.class.inc.php | 200 +++++++++++++++--- setup/compiler.class.inc.php | 1 + test/application/ThemeHandlerTest.php | 66 +++++- .../expected/css/_included_file3.scss | 0 .../expected/css/included_file1.scss | 0 .../css/included_scss/included_file2.scss | 0 .../expected/css/multi_imports.scss | 8 + .../theme-handler/expected/css/short_cut.scss | 2 + .../expected/css/simple_import.scss | 2 + .../expected/css/simple_import2.scss | 2 + .../expected/css/typography.scss | 0 11 files changed, 246 insertions(+), 35 deletions(-) create mode 100644 test/application/theme-handler/expected/css/_included_file3.scss create mode 100644 test/application/theme-handler/expected/css/included_file1.scss create mode 100644 test/application/theme-handler/expected/css/included_scss/included_file2.scss create mode 100644 test/application/theme-handler/expected/css/multi_imports.scss create mode 100644 test/application/theme-handler/expected/css/short_cut.scss create mode 100644 test/application/theme-handler/expected/css/simple_import.scss create mode 100644 test/application/theme-handler/expected/css/simple_import2.scss create mode 100644 test/application/theme-handler/expected/css/typography.scss diff --git a/application/themehandler.class.inc.php b/application/themehandler.class.inc.php index f4ca9baf8..6f7c420b2 100644 --- a/application/themehandler.class.inc.php +++ b/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 { /** diff --git a/setup/compiler.class.inc.php b/setup/compiler.class.inc.php index 5859384ee..b1096bd51 100644 --- a/setup/compiler.class.inc.php +++ b/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 diff --git a/test/application/ThemeHandlerTest.php b/test/application/ThemeHandlerTest.php index 466f1f99a..da20f860a 100644 --- a/test/application/ThemeHandlerTest.php +++ b/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 diff --git a/test/application/theme-handler/expected/css/_included_file3.scss b/test/application/theme-handler/expected/css/_included_file3.scss new file mode 100644 index 000000000..e69de29bb diff --git a/test/application/theme-handler/expected/css/included_file1.scss b/test/application/theme-handler/expected/css/included_file1.scss new file mode 100644 index 000000000..e69de29bb diff --git a/test/application/theme-handler/expected/css/included_scss/included_file2.scss b/test/application/theme-handler/expected/css/included_scss/included_file2.scss new file mode 100644 index 000000000..e69de29bb diff --git a/test/application/theme-handler/expected/css/multi_imports.scss b/test/application/theme-handler/expected/css/multi_imports.scss new file mode 100644 index 000000000..45e9b9057 --- /dev/null +++ b/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 +*/ diff --git a/test/application/theme-handler/expected/css/short_cut.scss b/test/application/theme-handler/expected/css/short_cut.scss new file mode 100644 index 000000000..c8b5b02c8 --- /dev/null +++ b/test/application/theme-handler/expected/css/short_cut.scss @@ -0,0 +1,2 @@ + +@import 'included_file3'; diff --git a/test/application/theme-handler/expected/css/simple_import.scss b/test/application/theme-handler/expected/css/simple_import.scss new file mode 100644 index 000000000..487af3a2f --- /dev/null +++ b/test/application/theme-handler/expected/css/simple_import.scss @@ -0,0 +1,2 @@ + +@import 'included_file1.scss'; diff --git a/test/application/theme-handler/expected/css/simple_import2.scss b/test/application/theme-handler/expected/css/simple_import2.scss new file mode 100644 index 000000000..735952d10 --- /dev/null +++ b/test/application/theme-handler/expected/css/simple_import2.scss @@ -0,0 +1,2 @@ + +@import 'included_scss/included_file2.scss'; diff --git a/test/application/theme-handler/expected/css/typography.scss b/test/application/theme-handler/expected/css/typography.scss new file mode 100644 index 000000000..e69de29bb