Merge pull request #4887 from nextcloud/theming-appdata-fix

Use appdata logo when generating icons

5 files changed, 67 insertions, 38 deletions

diff --git a/apps/theming/lib/IconBuilder.php b/apps/theming/lib/IconBuilder.php
index 7db24c4a2b0..42a7031f405 100644
--- a/apps/theming/lib/IconBuilder.php
+++ b/apps/theming/lib/IconBuilder.php
@@ -26,6 +26,7 @@ namespace OCA\Theming;
 use Imagick;
 use ImagickPixel;
 use OCP\App\AppPathNotFoundException;
+use OCP\Files\SimpleFS\ISimpleFile;
 
 class IconBuilder {
 	/** @var ThemingDefaults */
@@ -86,19 +87,23 @@ class IconBuilder {
 	 * @return Imagick|false
 	 */
 	public function renderAppIcon($app, $size) {
-		try {
-			$appIcon = $this->util->getAppIcon($app);
-			$appIconContent = file_get_contents($appIcon);
-		} catch (AppPathNotFoundException $e) {
+		$appIcon = $this->util->getAppIcon($app);
+		if($appIcon === false) {
 			return false;
 		}
+		if ($appIcon instanceof ISimpleFile) {
+			$appIconContent = $appIcon->getContent();
+			$mime = $appIcon->getMimeType();
+		} else {
+			$appIconContent = file_get_contents($appIcon);
+			$mime = mime_content_type($appIcon);
+		}
 
-		if($appIconContent === false) {
+		if($appIconContent === false || $appIconContent === "") {
 			return false;
 		}
 
 		$color = $this->themingDefaults->getColorPrimary();
-		$mime = mime_content_type($appIcon);
 
 		// generate background image with rounded corners
 		$background = '<?xml version="1.0" encoding="UTF-8"?>' .
@@ -136,10 +141,9 @@ class IconBuilder {
 		} else {
 			$appIconFile = new Imagick();
 			$appIconFile->setBackgroundColor(new ImagickPixel('transparent'));
-			$appIconFile->readImageBlob(file_get_contents($appIcon));
+			$appIconFile->readImageBlob($appIconContent);
 			$appIconFile->scaleImage(512, 512, true);
 		}
-
 		// offset for icon positioning
 		$border_w = (int)($appIconFile->getImageWidth() * 0.05);
 		$border_h = (int)($appIconFile->getImageHeight() * 0.05);
diff --git a/apps/theming/lib/Util.php b/apps/theming/lib/Util.php
index 5c9ccb3baa6..286756a4849 100644
--- a/apps/theming/lib/Util.php
+++ b/apps/theming/lib/Util.php
@@ -25,6 +25,9 @@ namespace OCA\Theming;
 
 use OCP\App\AppPathNotFoundException;
 use OCP\App\IAppManager;
+use OCP\Files\IAppData;
+use OCP\Files\NotFoundException;
+use OCP\Files\SimpleFS\ISimpleFile;
 use OCP\IConfig;
 use OCP\Files\IRootFolder;
 
@@ -33,23 +36,23 @@ class Util {
 	/** @var IConfig */
 	private $config;
 
-	/** @var IRootFolder */
-	private $rootFolder;
-
 	/** @var IAppManager */
 	private $appManager;
 
+	/** @var IAppData */
+	private $appData;
+
 	/**
 	 * Util constructor.
 	 *
 	 * @param IConfig $config
-	 * @param IRootFolder $rootFolder
 	 * @param IAppManager $appManager
+	 * @param IAppData $appData
 	 */
-	public function __construct(IConfig $config, IRootFolder $rootFolder, IAppManager $appManager) {
+	public function __construct(IConfig $config, IAppManager $appManager, IAppData $appData) {
 		$this->config = $config;
-		$this->rootFolder = $rootFolder;
 		$this->appManager = $appManager;
+		$this->appData = $appData;
 	}
 
 	/**
@@ -111,7 +114,7 @@ class Util {
 
 	/**
 	 * @param $app string app name
-	 * @return string path to app icon / logo
+	 * @return string|ISimpleFile path to app icon / file of logo
 	 */
 	public function getAppIcon($app) {
 		$app = str_replace(array('\0', '/', '\\', '..'), '', $app);
@@ -127,8 +130,14 @@ class Util {
 			}
 		} catch (AppPathNotFoundException $e) {}
 
-		if($this->config->getAppValue('theming', 'logoMime', '') !== '' && $this->rootFolder->nodeExists('/themedinstancelogo')) {
-			return $this->config->getSystemValue('datadirectory', \OC::$SERVERROOT . '/data') . '/themedinstancelogo';
+		if ($this->config->getAppValue('theming', 'logoMime', '') !== '') {
+			$logoFile = null;
+			try {
+				$folder = $this->appData->getFolder('images');
+				if ($folder !== null) {
+					return $folder->getFile('logo');
+				}
+			} catch (NotFoundException $e) {}
 		}
 		return \OC::$SERVERROOT . '/core/img/logo.svg';
 	}
diff --git a/apps/theming/tests/Controller/ThemingControllerTest.php b/apps/theming/tests/Controller/ThemingControllerTest.php
index cbdb86d0358..5fa8dc51939 100644
--- a/apps/theming/tests/Controller/ThemingControllerTest.php
+++ b/apps/theming/tests/Controller/ThemingControllerTest.php
@@ -62,8 +62,6 @@ class ThemingControllerTest extends TestCase {
 	private $l10n;
 	/** @var ThemingController */
 	private $themingController;
-	/** @var IRootFolder|\PHPUnit_Framework_MockObject_MockObject */
-	private $rootFolder;
 	/** @var ITempManager */
 	private $tempManager;
 	/** @var IAppManager|\PHPUnit_Framework_MockObject_MockObject */
@@ -79,14 +77,13 @@ class ThemingControllerTest extends TestCase {
 		$this->themingDefaults = $this->createMock(ThemingDefaults::class);
 		$this->timeFactory = $this->createMock(ITimeFactory::class);
 		$this->l10n = $this->createMock(L10N::class);
-		$this->rootFolder = $this->createMock(IRootFolder::class);
+		$this->appData = $this->createMock(IAppData::class);
 		$this->appManager = $this->createMock(IAppManager::class);
-		$this->util = new Util($this->config, $this->rootFolder, $this->appManager);
+		$this->util = new Util($this->config, $this->appManager, $this->appData);
 		$this->timeFactory->expects($this->any())
 			->method('getTime')
 			->willReturn(123);
 		$this->tempManager = \OC::$server->getTempManager();
-		$this->appData = $this->createMock(IAppData::class);
 		$this->scssCacher = $this->createMock(SCSSCacher::class);
 
 		$this->themingController = new ThemingController(
diff --git a/apps/theming/tests/IconBuilderTest.php b/apps/theming/tests/IconBuilderTest.php
index 423e3e86dbc..9b5b1933201 100644
--- a/apps/theming/tests/IconBuilderTest.php
+++ b/apps/theming/tests/IconBuilderTest.php
@@ -27,7 +27,9 @@ use OCA\Theming\ThemingDefaults;
 use OCA\Theming\Util;
 use OCP\App\IAppManager;
 use OCP\AppFramework\Http\NotFoundResponse;
+use OCP\Files\IAppData;
 use OCP\Files\IRootFolder;
+use OCP\Files\NotFoundException;
 use OCP\IConfig;
 use Test\TestCase;
 
@@ -35,8 +37,8 @@ class IconBuilderTest extends TestCase {
 
 	/** @var IConfig */
 	protected $config;
-	/** @var IRootFolder */
-	protected $rootFolder;
+	/** @var IAppData */
+	protected $appData;
 	/** @var ThemingDefaults */
 	protected $themingDefaults;
 	/** @var Util */
@@ -50,11 +52,11 @@ class IconBuilderTest extends TestCase {
 		parent::setUp();
 
 		$this->config = $this->getMockBuilder('\OCP\IConfig')->getMock();
-		$this->rootFolder = $this->getMockBuilder('OCP\Files\IRootFolder')->getMock();
+		$this->appData = $this->createMock(IAppData::class);
 		$this->themingDefaults = $this->getMockBuilder('OCA\Theming\ThemingDefaults')
 			->disableOriginalConstructor()->getMock();
 		$this->appManager = $this->getMockBuilder('OCP\App\IAppManager')->getMock();
-		$this->util = new Util($this->config, $this->rootFolder, $this->appManager);
+		$this->util = new Util($this->config, $this->appManager, $this->appData);
 		$this->iconBuilder = new IconBuilder($this->themingDefaults, $this->util);
 	}
 
@@ -89,6 +91,10 @@ class IconBuilderTest extends TestCase {
 		$this->themingDefaults->expects($this->once())
 			->method('getColorPrimary')
 			->willReturn($color);
+		$this->appData->expects($this->once())
+			->method('getFolder')
+			->with('images')
+			->willThrowException(new NotFoundException());
 
 		$expectedIcon = new \Imagick(realpath(dirname(__FILE__)). "/data/" . $file);
 		$icon = $this->iconBuilder->renderAppIcon($app, 512);
diff --git a/apps/theming/tests/UtilTest.php b/apps/theming/tests/UtilTest.php
index 83895208fea..de6690ffe0d 100644
--- a/apps/theming/tests/UtilTest.php
+++ b/apps/theming/tests/UtilTest.php
@@ -24,6 +24,10 @@ namespace OCA\Theming\Tests;
 
 use OCA\Theming\Util;
 use OCP\App\IAppManager;
+use OCP\Files\IAppData;
+use OCP\Files\NotFoundException;
+use OCP\Files\SimpleFS\ISimpleFile;
+use OCP\Files\SimpleFS\ISimpleFolder;
 use OCP\IConfig;
 use OCP\Files\IRootFolder;
 use Test\TestCase;
@@ -34,17 +38,17 @@ class UtilTest extends TestCase {
 	protected $util;
 	/** @var IConfig */
 	protected $config;
-	/** @var IRootFolder */
-	protected $rootFolder;
+	/** @var IAppData */
+	protected $appData;
 	/** @var IAppManager */
 	protected $appManager;
 
 	protected function setUp() {
 		parent::setUp();
-		$this->config = $this->getMockBuilder('\OCP\IConfig')->getMock();
-		$this->rootFolder = $this->getMockBuilder('OCP\Files\IRootFolder')->getMock();
-		$this->appManager = $this->getMockBuilder('OCP\App\IAppManager')->getMock();
-		$this->util = new Util($this->config, $this->rootFolder, $this->appManager);
+		$this->config = $this->createMock(IConfig::class);
+		$this->appData = $this->createMock(IAppData::class);
+		$this->appManager = $this->createMock(IAppManager::class);
+		$this->util = new Util($this->config, $this->appManager, $this->appData);
 	}
 
 	public function testInvertTextColorLight() {
@@ -112,6 +116,10 @@ class UtilTest extends TestCase {
 	 * @dataProvider dataGetAppIcon
 	 */
 	public function testGetAppIcon($app, $expected) {
+		$this->appData->expects($this->any())
+			->method('getFolder')
+			->with('images')
+			->willThrowException(new NotFoundException());
 		$this->appManager->expects($this->once())
 			->method('getAppPath')
 			->with($app)
@@ -129,13 +137,18 @@ class UtilTest extends TestCase {
 	}
 
 	public function testGetAppIconThemed() {
-		$this->rootFolder->expects($this->once())
-			->method('nodeExists')
-			->with('/themedinstancelogo')
-			->willReturn(true);
-		$expected = '/themedinstancelogo';
+		$file = $this->createMock(ISimpleFile::class);
+		$folder = $this->createMock(ISimpleFolder::class);
+		$folder->expects($this->once())
+			->method('getFile')
+			->with('logo')
+			->willReturn($file);
+		$this->appData->expects($this->once())
+			->method('getFolder')
+			->with('images')
+			->willReturn($folder);
 		$icon = $this->util->getAppIcon('noapplikethis');
-		$this->assertEquals($expected, $icon);
+		$this->assertEquals($file, $icon);
 	}
 
 	/**