Merge pull request #8999 from nextcloud/css-js-name-based-on-apps-versions

Use app version to generate scss filename

lib/private/Template/SCSSCacher.php

@@ -112,7 +112,7 @@ class SCSSCacher {
 		$path = explode('/', $root . '/' . $file);
 
 		$fileNameSCSS = array_pop($path);
-		$fileNameCSS = $this->prependBaseurlPrefix(str_replace('.scss', '.css', $fileNameSCSS));
+		$fileNameCSS = $this->prependVersionPrefix($this->prependBaseurlPrefix(str_replace('.scss', '.css', $fileNameSCSS)), $app);
 
 		$path = implode('/', $path);
 		$webDir = $this->getWebDir($path, $app, $this->serverRoot, \OC::$WEBROOT);
@@ -138,7 +138,8 @@ class SCSSCacher {
 	 */
 	public function getCachedCSS(string $appName, string $fileName): ISimpleFile {
 		$folder = $this->appData->getFolder($appName);
-		return $folder->getFile($this->prependBaseurlPrefix($fileName));
+		$cachedFileName = $this->prependVersionPrefix($this->prependBaseurlPrefix($fileName), $appName);
+		return $folder->getFile($cachedFileName);
 	}
 
 	/**
@@ -322,19 +323,34 @@ class SCSSCacher {
 	public function getCachedSCSS(string $appName, string $fileName): string {
 		$tmpfileLoc = explode('/', $fileName);
 		$fileName = array_pop($tmpfileLoc);
-		$fileName = $this->prependBaseurlPrefix(str_replace('.scss', '.css', $fileName));
+		$fileName = $this->prependVersionPrefix($this->prependBaseurlPrefix(str_replace('.scss', '.css', $fileName)), $appName);
 
 		return substr($this->urlGenerator->linkToRoute('core.Css.getCss', ['fileName' => $fileName, 'appName' => $appName]), strlen(\OC::$WEBROOT) + 1);
 	}
 
 	/**
 	 * Prepend hashed base url to the css file
-	 * @param string$cssFile
+	 * @param string $cssFile
 	 * @return string
 	 */
 	private function prependBaseurlPrefix(string $cssFile): string {
 		$frontendController = ($this->config->getSystemValue('htaccess.IgnoreFrontController', false) === true || getenv('front_controller_active') === 'true');
-		return substr(md5($this->urlGenerator->getBaseUrl() . $frontendController), 0, 8) . '-' . $cssFile;
+		return substr(md5($this->urlGenerator->getBaseUrl() . $frontendController), 0, 4) . '-' . $cssFile;
+	}
+
+	/**
+	 * Prepend hashed app version hash
+	 * @param string $cssFile
+	 * @param string $appId
+	 * @return string
+	 */
+	private function prependVersionPrefix(string $cssFile, string $appId): string {
+		$appVersion = \OC_App::getAppVersion($appId);
+		if ($appVersion !== '0') {
+			return substr(md5($appVersion), 0, 4) . '-' . $cssFile;
+		}
+		$coreVersion = \OC_Util::getVersionString();
+		return substr(md5($coreVersion), 0, 4) . '-' . $cssFile;
 	}
 
 	/**

tests/lib/Template/SCSSCacherTest.php

@@ -35,6 +35,7 @@ use OCP\ICacheFactory;
 use OCP\IConfig;
 use OCP\ILogger;
 use OCP\IURLGenerator;
+use OC_App;
 
 class SCSSCacherTest extends \Test\TestCase {
 	/** @var ILogger|\PHPUnit_Framework_MockObject_MockObject */
@@ -58,17 +59,26 @@ class SCSSCacherTest extends \Test\TestCase {
 		parent::setUp();
 		$this->logger = $this->createMock(ILogger::class);
 		$this->appData = $this->createMock(IAppData::class);
+
 		/** @var Factory|\PHPUnit_Framework_MockObject_MockObject $factory */
 		$factory = $this->createMock(Factory::class);
 		$factory->method('get')->with('css')->willReturn($this->appData);
+
 		$this->urlGenerator = $this->createMock(IURLGenerator::class);
+		$this->urlGenerator->expects($this->any())
+			->method('getBaseUrl')
+			->willReturn('http://localhost/nextcloud');
+
 		$this->config = $this->createMock(IConfig::class);
 		$this->cacheFactory = $this->createMock(ICacheFactory::class);
 		$this->depsCache = $this->createMock(ICache::class);
 		$this->cacheFactory->expects($this->at(0))
 			->method('createDistributed')
 			->willReturn($this->depsCache);
+
 		$this->themingDefaults = $this->createMock(ThemingDefaults::class);
+		$this->themingDefaults->expects($this->any())->method('getScssVariables')->willReturn([]);
+
 		$this->scssCacher = new SCSSCacher(
 			$this->logger,
 			$factory,
@@ -78,11 +88,6 @@ class SCSSCacherTest extends \Test\TestCase {
 			\OC::$SERVERROOT,
 			$this->cacheFactory
 		);
-		$this->themingDefaults->expects($this->any())->method('getScssVariables')->willReturn([]);
-
-		$this->urlGenerator->expects($this->any())
-			->method('getBaseUrl')
-			->willReturn('http://localhost/nextcloud');
 	}
 
 	public function testProcessUncachedFileNoAppDataFolder() {
@@ -96,7 +101,8 @@ class SCSSCacherTest extends \Test\TestCase {
 
 		$fileDeps = $this->createMock(ISimpleFile::class);
 		$gzfile = $this->createMock(ISimpleFile::class);
-		$filePrefix = substr(md5('http://localhost/nextcloud'), 0, 8) . '-';
+		$filePrefix = substr(md5(\OC_Util::getVersionString('core')), 0, 4) . '-' .
+					  substr(md5('http://localhost/nextcloud'), 0, 4) . '-';
 
 		$folder->method('getFile')
 			->will($this->returnCallback(function($path) use ($file, $gzfile, $filePrefix) {
@@ -131,7 +137,8 @@ class SCSSCacherTest extends \Test\TestCase {
 		$file->expects($this->any())->method('getSize')->willReturn(1);
 		$fileDeps = $this->createMock(ISimpleFile::class);
 		$gzfile = $this->createMock(ISimpleFile::class);
-		$filePrefix = substr(md5('http://localhost/nextcloud'), 0, 8) . '-';
+		$filePrefix = substr(md5(\OC_Util::getVersionString('core')), 0, 4) . '-' .
+					  substr(md5('http://localhost/nextcloud'), 0, 4) . '-';
 
 		$folder->method('getFile')
 			->will($this->returnCallback(function($path) use ($file, $gzfile, $filePrefix) {
@@ -162,7 +169,8 @@ class SCSSCacherTest extends \Test\TestCase {
 		$fileDeps = $this->createMock(ISimpleFile::class);
 		$fileDeps->expects($this->any())->method('getSize')->willReturn(1);
 		$gzFile = $this->createMock(ISimpleFile::class);
-		$filePrefix = substr(md5('http://localhost/nextcloud'), 0, 8) . '-';
+		$filePrefix = substr(md5(\OC_Util::getVersionString('core')), 0, 4) . '-' .
+					  substr(md5('http://localhost/nextcloud'), 0, 4) . '-';
 
 		$folder->method('getFile')
 			->will($this->returnCallback(function($name) use ($file, $fileDeps, $gzFile, $filePrefix) {
@@ -197,6 +205,8 @@ class SCSSCacherTest extends \Test\TestCase {
 
 		$gzFile = $this->createMock(ISimpleFile::class);
 		$filePrefix = substr(md5('http://localhost/nextcloud'), 0, 8) . '-';
+		$filePrefix = substr(md5(\OC_Util::getVersionString('core')), 0, 4) . '-' .
+					  substr(md5('http://localhost/nextcloud'), 0, 4) . '-';
 		$folder->method('getFile')
 			->will($this->returnCallback(function($name) use ($file, $fileDeps, $gzFile, $filePrefix) {
 				if ($name === $filePrefix.'styles.css') {
@@ -382,8 +392,8 @@ class SCSSCacherTest extends \Test\TestCase {
 
 	public function dataGetCachedSCSS() {
 		return [
-			['core', 'core/css/styles.scss', '/css/core/styles.css'],
+			['core', 'core/css/styles.scss', '/css/core/styles.css', \OC_Util::getVersionString()],
-			['files', 'apps/files/css/styles.scss', '/css/files/styles.css']
+			['files', 'apps/files/css/styles.scss', '/css/files/styles.css', \OC_App::getAppVersion('files')]
 		];
 	}
 
@@ -393,11 +403,12 @@ class SCSSCacherTest extends \Test\TestCase {
 	 * @param $result
 	 * @dataProvider dataGetCachedSCSS
 	 */
-	public function testGetCachedSCSS($appName, $fileName, $result) {
+	public function testGetCachedSCSS($appName, $fileName, $result, $version) {
 		$this->urlGenerator->expects($this->once())
 			->method('linkToRoute')
 			->with('core.Css.getCss', [
-				'fileName' => substr(md5('http://localhost/nextcloud'), 0, 8) . '-styles.css',
+				'fileName' => substr(md5($version), 0, 4) . '-' .
+							  substr(md5('http://localhost/nextcloud'), 0, 4) . '-styles.css',
 				'appName' => $appName
 			])
 			->willReturn(\OC::$WEBROOT . $result);