Merge pull request #5719 from nextcloud/improve-jscombiner

Properly handle if the deps file if for some reason empty

lib/private/Template/JSCombiner.php

@@ -28,6 +28,7 @@ use OCP\Files\IAppData;
 use OCP\Files\NotFoundException;
 use OCP\Files\NotPermittedException;
 use OCP\Files\SimpleFS\ISimpleFolder;
+use OCP\ILogger;
 use OCP\IURLGenerator;
 
 class JSCombiner {
@@ -44,20 +45,26 @@ class JSCombiner {
 	/** @var SystemConfig */
 	protected $config;
 
+	/** @var ILogger */
+	protected $logger;
+
 	/**
 	 * @param IAppData $appData
 	 * @param IURLGenerator $urlGenerator
 	 * @param ICache $depsCache
 	 * @param SystemConfig $config
+	 * @param ILogger $logger
 	 */
 	public function __construct(IAppData $appData,
 								IURLGenerator $urlGenerator,
 								ICache $depsCache,
-								SystemConfig $config) {
+								SystemConfig $config,
+								ILogger $logger) {
 		$this->appData = $appData;
 		$this->urlGenerator = $urlGenerator;
 		$this->depsCache = $depsCache;
 		$this->config = $config;
+		$this->logger = $logger;
 	}
 
 	/**
@@ -102,6 +109,12 @@ class JSCombiner {
 				$depFile = $folder->getFile($fileName);
 				$deps = $depFile->getContent();
 			}
+			// check again
+			if ($deps === null || $deps === '') {
+				$this->logger->info('JSCombiner: deps file empty: ' . $fileName);
+				return false;
+			}
+
 			$deps = json_decode($deps, true);
 
 			foreach ($deps as $file=>$mtime) {

lib/private/TemplateLayout.php

@@ -243,7 +243,8 @@ class TemplateLayout extends \OC_Template {
 				\OC::$server->getAppDataDir('js'),
 				\OC::$server->getURLGenerator(),
 				\OC::$server->getMemCacheFactory()->create('JS'),
-				\OC::$server->getSystemConfig()
+				\OC::$server->getSystemConfig(),
+				\OC::$server->getLogger()
 			)
 			);
 		$locator->find($scripts);

tests/lib/Template/JSCombinerTest.php

@@ -31,6 +31,7 @@ use OCP\Files\NotPermittedException;
 use OCP\Files\SimpleFS\ISimpleFile;
 use OCP\Files\SimpleFS\ISimpleFolder;
 use OCP\ICache;
+use OCP\ILogger;
 use OCP\IURLGenerator;
 
 class JSCombinerTest extends \Test\TestCase {
@@ -44,6 +45,8 @@ class JSCombinerTest extends \Test\TestCase {
 	protected $depsCache;
 	/** @var JSCombiner */
 	protected $jsCombiner;
+	/** @var ILogger|\PHPUnit_Framework_MockObject_MockObject */
+	protected $logger;
 
 	protected function setUp() {
 		parent::setUp();
@@ -52,11 +55,14 @@ class JSCombinerTest extends \Test\TestCase {
 		$this->urlGenerator = $this->createMock(IURLGenerator::class);
 		$this->config = $this->createMock(SystemConfig::class);
 		$this->depsCache = $this->createMock(ICache::class);
+		$this->logger = $this->createMock(ILogger::class);
 		$this->jsCombiner = new JSCombiner(
 			$this->appData,
 			$this->urlGenerator,
 			$this->depsCache,
-			$this->config);
+			$this->config,
+			$this->logger
+		);
 	}
 
 	public function testProcessDebugMode() {
@@ -284,6 +290,23 @@ class JSCombinerTest extends \Test\TestCase {
 		$this->assertFalse($actual);
 	}
 
+	public function testIsCachedWithoutContent() {
+		$fileName = 'combine.json';
+		$folder = $this->createMock(ISimpleFolder::class);
+		$file = $this->createMock(ISimpleFile::class);
+		$folder->method('getFile')
+			->with('combine.js.deps')
+			->willReturn($file);
+		$file->expects($this->once())
+			->method('getContent')
+			->willReturn('');
+		$this->logger->expects($this->once())
+			->method('info')
+			->with('JSCombiner: deps file empty: combine.js.deps');
+		$actual = self::invokePrivate($this->jsCombiner, 'isCached', [$fileName, $folder]);
+		$this->assertFalse($actual);
+	}
+
 	public function testCacheNoFile() {
 		$fileName = 'combine.js';