Skip to content

Lock SCSS so we only run 1 job at a time #15794

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rullzer merged 1 commit into from
Jul 12, 2019
Merged

Lock SCSS so we only run 1 job at a time #15794

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions lib/private/Template/IconsCacher.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,9 @@ public function getCachedList() {
}
}

/**
* Add the icons cache css into the header
*/
public function injectCss() {
$mtime = $this->timeFactory->getTime();
$file = $this->getCachedList();
Expand Down
92 changes: 74 additions & 18 deletions lib/private/Template/SCSSCacher.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
use Leafo\ScssPhp\Exception\ParserException;
use Leafo\ScssPhp\Formatter\Crunched;
use Leafo\ScssPhp\Formatter\Expanded;
use OC\Memcache\NullCache;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\Files\IAppData;
use OCP\Files\NotFoundException;
Expand All @@ -42,6 +43,7 @@
use OCP\ICacheFactory;
use OCP\IConfig;
use OCP\ILogger;
use OCP\IMemcache;
use OCP\IURLGenerator;
use OC\Files\AppData\Factory;
use OC\Template\IconsCacher;
Expand Down Expand Up @@ -84,6 +86,9 @@ class SCSSCacher {
/** @var ITimeFactory */
private $timeFactory;

/** @var IMemcache */
private $lockingCache;

/**
* @param ILogger $logger
* @param Factory $appDataFactory
Expand Down Expand Up @@ -111,8 +116,13 @@ public function __construct(ILogger $logger,
$this->defaults = $defaults;
$this->serverRoot = $serverRoot;
$this->cacheFactory = $cacheFactory;
$this->depsCache = $cacheFactory->createDistributed('SCSS-' . md5($this->urlGenerator->getBaseUrl()));
$this->depsCache = $cacheFactory->createDistributed('SCSS-deps-' . md5($this->urlGenerator->getBaseUrl()));
$this->isCachedCache = $cacheFactory->createLocal('SCSS-cached-' . md5($this->urlGenerator->getBaseUrl()));
$lockingCache = $cacheFactory->createDistributed('SCSS-locks-' . md5($this->urlGenerator->getBaseUrl()));
if (!($lockingCache instanceof IMemcache)) {
$lockingCache = new NullCache();
}
$this->lockingCache = $lockingCache;
$this->iconsCacher = $iconsCacher;
$this->timeFactory = $timeFactory;
}
Expand All @@ -137,10 +147,7 @@ public function process(string $root, string $file, string $app): bool {

if (!$this->variablesChanged() && $this->isCached($fileNameCSS, $app)) {
// Inject icons vars css if any
if ($this->iconsCacher->getCachedCSS() && $this->iconsCacher->getCachedCSS()->getSize() > 0) {
$this->iconsCacher->injectCss();
}
return true;
return $this->injectCssVariablesIfAny();
}

try {
Expand All @@ -150,7 +157,35 @@ public function process(string $root, string $file, string $app): bool {
$folder = $this->appData->newFolder($app);
}

$cached = $this->cache($path, $fileNameCSS, $fileNameSCSS, $folder, $webDir);
$lockKey = $webDir . '/' . $fileNameSCSS;

if (!$this->lockingCache->add($lockKey, 'locked!', 120)) {
$retry = 0;
sleep(1);
while ($retry < 10) {
if (!$this->variablesChanged() && $this->isCached($fileNameCSS, $app)) {
// Inject icons vars css if any
$this->lockingCache->remove($lockKey);
$this->logger->debug('SCSSCacher: ' .$lockKey.' is now available after '.$retry.'s. Moving on...', ['app' => 'core']);
return $this->injectCssVariablesIfAny();
}
$this->logger->debug('SCSSCacher: scss cache file locked for '.$lockKey, ['app' => 'core']);
sleep($retry);
$retry++;
}
$this->logger->debug('SCSSCacher: Giving up scss caching for '.$lockKey, ['app' => 'core']);
return false;
}

try {
$cached = $this->cache($path, $fileNameCSS, $fileNameSCSS, $folder, $webDir);
} catch (\Exception $e) {
$this->lockingCache->remove($lockKey);
throw $e;
}

// Cleaning lock
$this->lockingCache->remove($lockKey);

// Inject icons vars css if any
if ($this->iconsCacher->getCachedCSS() && $this->iconsCacher->getCachedCSS()->getSize() > 0) {
Expand Down Expand Up @@ -180,19 +215,24 @@ public function getCachedCSS(string $appName, string $fileName): ISimpleFile {
*/
private function isCached(string $fileNameCSS, string $app) {
$key = $this->config->getSystemValue('version') . '/' . $app . '/' . $fileNameCSS;
if (!$this->config->getSystemValue('debug') && $cacheValue = $this->isCachedCache->get($key)) {

// If the file mtime is more recent than our cached one,
// let's consider the file is properly cached
if ($cacheValue = $this->isCachedCache->get($key)) {
if ($cacheValue > $this->timeFactory->getTime()) {
return true;
}
}

// Creating file cache if none for further checks
try {
$folder = $this->appData->getFolder($app);
} catch (NotFoundException $e) {
// creating css appdata folder
$folder = $this->appData->newFolder($app);
return false;
}

// Checking if file size is coherent
// and if one of the css dependency changed
try {
$cachedFile = $folder->getFile($fileNameCSS);
if ($cachedFile->getSize() > 0) {
Expand All @@ -201,7 +241,7 @@ private function isCached(string $fileNameCSS, string $app) {
if ($deps === null) {
$depFile = $folder->getFile($depFileName);
$deps = $depFile->getContent();
//Set to memcache for next run
// Set to memcache for next run
$this->depsCache->set($folder->getName() . '-' . $depFileName, $deps);
}
$deps = json_decode($deps, true);
Expand All @@ -228,13 +268,11 @@ private function isCached(string $fileNameCSS, string $app) {
*/
private function variablesChanged(): bool {
$injectedVariables = $this->getInjectedVariables();
if ($this->config->getAppValue('core', 'scss.variables') !== md5($injectedVariables)) {
if ($this->config->getAppValue('core', 'theming.variables') !== md5($injectedVariables)) {
$this->resetCache();
$this->config->setAppValue('core', 'scss.variables', md5($injectedVariables));

$this->config->setAppValue('core', 'theming.variables', md5($injectedVariables));
return true;
}

return false;
}

Expand Down Expand Up @@ -289,7 +327,7 @@ private function cache(string $path, string $fileNameCSS, string $fileNameSCSS,
'@import "functions.scss";' .
'@import "' . $fileNameSCSS . '";');
} catch (ParserException $e) {
$this->logger->error($e, ['app' => 'core']);
$this->logger->logException($e, ['app' => 'core']);

return false;
}
Expand Down Expand Up @@ -327,7 +365,11 @@ private function cache(string $path, string $fileNameCSS, string $fileNameSCSS,
*/
public function resetCache() {
$this->injectedVariables = null;
$this->cacheFactory->createDistributed('SCSS-')->clear();

// do not clear locks
$this->cacheFactory->createDistributed('SCSS-deps-')->clear();
$this->cacheFactory->createDistributed('SCSS-cached-')->clear();

$appDirectory = $this->appData->getDirectoryListing();
foreach ($appDirectory as $folder) {
foreach ($folder->getDirectoryListing() as $file) {
Expand All @@ -338,6 +380,7 @@ public function resetCache() {
}
}
}
$this->logger->debug('SCSSCacher: css cache cleared!');
}

/**
Expand All @@ -358,7 +401,7 @@ private function getInjectedVariables(): string {
$scss->compile($variables);
$this->injectedVariables = $variables;
} catch (ParserException $e) {
$this->logger->error($e, ['app' => 'core']);
$this->logger->logException($e, ['app' => 'core']);
}

return $variables;
Expand Down Expand Up @@ -391,7 +434,7 @@ public function getCachedSCSS(string $appName, string $fileName): string {
return substr($this->urlGenerator->linkToRoute('core.Css.getCss', [
'fileName' => $fileName,
'appName' => $appName,
'v' => $this->config->getAppValue('core', 'scss.variables', '0')
'v' => $this->config->getAppValue('core', 'theming.variables', '0')
]), \strlen(\OC::$WEBROOT) + 1);
}

Expand Down Expand Up @@ -449,4 +492,17 @@ private function getWebDir(string $path, string $appName, string $serverRoot, st

return $webRoot . substr($path, strlen($serverRoot));
}

/**
* Add the icons css cache in the header if needed
*
* @return boolean true
*/
private function injectCssVariablesIfAny() {
// Inject icons vars css if any
if ($this->iconsCacher->getCachedCSS() && $this->iconsCacher->getCachedCSS()->getSize() > 0) {
$this->iconsCacher->injectCss();
}
return true;
}
}
4 changes: 2 additions & 2 deletions tests/lib/Template/SCSSCacherTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -528,10 +528,10 @@ public function testResetCache() {
->willReturn([$file]);

$cache = $this->createMock(ICache::class);
$this->cacheFactory->expects($this->once())
$this->cacheFactory->expects($this->exactly(2))
->method('createDistributed')
->willReturn($cache);
$cache->expects($this->once())
$cache->expects($this->exactly(2))
->method('clear')
->with('');
$this->appData->expects($this->once())
Expand Down