Fix theming tests

Signed-off-by: Julius Härtl <jus@bitgrid.net>

apps/theming/lib/Controller/ThemingController.php

@@ -100,6 +100,7 @@ class ThemingController extends Controller {
 	 * @param SCSSCacher $scssCacher
 	 * @param IURLGenerator $urlGenerator
 	 * @param IAppManager $appManager
+	 * @param ImageManager $imageManager
 	 */
 	public function __construct(
 		$appName,
@@ -313,12 +314,7 @@ class ThemingController extends Controller {
 
 		if (strpos($setting, 'Mime') !== -1) {
 			$imageKey = str_replace('Mime', '', $setting);
-			try {
-				$file = $this->appData->getFolder('images')->getFile($imageKey);
-				$file->delete();
-			} catch (NotFoundException $e) {
-			} catch (NotPermittedException $e) {
-			}
+			$this->imageManager->delete($imageKey);
 		}
 
 		return new DataResponse(
@@ -344,8 +340,7 @@ class ThemingController extends Controller {
 	 */
 	public function getImage(string $key) {
 		try {
-			/** @var File $file */
-			$file = $this->appData->getFolder('images')->getFile($key);
+			$file = $this->imageManager->getImage($key);
 		} catch (NotFoundException $e) {
 			return new NotFoundResponse();
 		}

apps/theming/lib/ImageManager.php

@@ -158,8 +158,20 @@ class ImageManager {
 		return $file;
 	}
 
+	public function delete(string $key) {
+		try {
+			$file = $this->appData->getFolder('images')->getFile($key);
+			$file->delete();
+		} catch (NotFoundException $e) {
+		} catch (NotPermittedException $e) {
+		}
+	}
+
 	/**
 	 * remove cached files that are not required any longer
+	 *
+	 * @throws NotPermittedException
+	 * @throws NotFoundException
 	 */
 	public function cleanup() {
 		$currentFolder = $this->getCacheFolder();

apps/theming/tests/Controller/IconControllerTest.php

@@ -33,7 +33,6 @@ use OCA\Theming\ImageManager;
 use OCA\Theming\ThemingDefaults;
 use OCP\AppFramework\Http;
 use OCP\AppFramework\Http\DataDisplayResponse;
-use OCP\AppFramework\Http\NotFoundResponse;
 use OCP\Files\NotFoundException;
 use OCP\IConfig;
 use OCP\IRequest;
@@ -41,6 +40,7 @@ use Test\TestCase;
 use OCA\Theming\Util;
 use OCA\Theming\Controller\IconController;
 use OCP\AppFramework\Http\FileDisplayResponse;
+use OCP\AppFramework\Utility\ITimeFactory;
 
 
 class IconControllerTest extends TestCase {
@@ -48,8 +48,6 @@ class IconControllerTest extends TestCase {
 	private $request;
 	/** @var ThemingDefaults|\PHPUnit_Framework_MockObject_MockObject */
 	private $themingDefaults;
-	/** @var Util */
-	private $util;
 	/** @var \OCP\AppFramework\Utility\ITimeFactory */
 	private $timeFactory;
 	/** @var IconController|\PHPUnit_Framework_MockObject_MockObject */
@@ -64,18 +62,11 @@ class IconControllerTest extends TestCase {
 	private $imageManager;
 
 	public function setUp() {
-		$this->request = $this->getMockBuilder(IRequest::class)->getMock();
-		$this->themingDefaults = $this->getMockBuilder('OCA\Theming\ThemingDefaults')
-			->disableOriginalConstructor()->getMock();
-		$this->util = $this->getMockBuilder('\OCA\Theming\Util')->disableOriginalConstructor()
-			->setMethods(['getAppImage', 'getAppIcon', 'elementColor'])->getMock();
-		$this->timeFactory = $this->getMockBuilder('OCP\AppFramework\Utility\ITimeFactory')
-			->disableOriginalConstructor()
-			->getMock();
-		$this->config = $this->getMockBuilder(IConfig::class)->getMock();
-		$this->iconBuilder = $this->getMockBuilder('OCA\Theming\IconBuilder')
-			->disableOriginalConstructor()->getMock();
-		$this->imageManager = $this->getMockBuilder('OCA\Theming\ImageManager')->disableOriginalConstructor()->getMock();
+		$this->request = $this->createMock(IRequest::class);
+		$this->themingDefaults = $this->createMock(ThemingDefaults::class);
+		$this->timeFactory = $this->createMock(ITimeFactory::class);
+		$this->iconBuilder = $this->createMock(IconBuilder::class);
+		$this->imageManager = $this->createMock(ImageManager::class);
 		$this->fileAccessHelper = $this->createMock(FileAccessHelper::class);
 		$this->timeFactory->expects($this->any())
 			->method('getTime')
@@ -85,9 +76,7 @@ class IconControllerTest extends TestCase {
 			'theming',
 			$this->request,
 			$this->themingDefaults,
-			$this->util,
 			$this->timeFactory,
-			$this->config,
 			$this->iconBuilder,
 			$this->imageManager,
 			$this->fileAccessHelper
@@ -129,18 +118,21 @@ class IconControllerTest extends TestCase {
 		if (count($checkImagick->queryFormats('SVG')) < 1) {
 			$this->markTestSkipped('No SVG provider present.');
 		}
+		$file = $this->iconFileMock('filename', 'filecontent');
+		$this->imageManager->expects($this->once())
+			->method('getImage')
+			->with('favicon')
+			->will($this->throwException(new NotFoundException()));
 		$this->themingDefaults->expects($this->any())
 			->method('shouldReplaceIcons')
 			->willReturn(true);
-
+		$this->imageManager->expects($this->once())
+			->method('getCachedImage')
+			->will($this->throwException(new NotFoundException()));
 		$this->iconBuilder->expects($this->once())
 			->method('getFavicon')
 			->with('core')
 			->willReturn('filecontent');
-		$file = $this->iconFileMock('filename', 'filecontent');
-		$this->imageManager->expects($this->once())
-			->method('getCachedImage')
-			->will($this->throwException(new NotFoundException()));
 		$this->imageManager->expects($this->once())
 			->method('setCachedImage')
 			->willReturn($file);
@@ -156,6 +148,10 @@ class IconControllerTest extends TestCase {
 	}
 
 	public function testGetFaviconFail() {
+		$this->imageManager->expects($this->once())
+			->method('getImage')
+			->with('favicon')
+			->will($this->throwException(new NotFoundException()));
 		$this->themingDefaults->expects($this->any())
 			->method('shouldReplaceIcons')
 			->willReturn(false);
@@ -209,6 +205,10 @@ class IconControllerTest extends TestCase {
 	}
 
 	public function testGetTouchIconFail() {
+		$this->imageManager->expects($this->once())
+			->method('getImage')
+			->with('favicon')
+			->will($this->throwException(new NotFoundException()));
 		$this->themingDefaults->expects($this->any())
 			->method('shouldReplaceIcons')
 			->willReturn(false);

apps/theming/tests/Controller/ThemingControllerTest.php

@@ -34,6 +34,7 @@ use OC\Files\AppData\Factory;
 use OC\L10N\L10N;
 use OC\Template\SCSSCacher;
 use OCA\Theming\Controller\ThemingController;
+use OCA\Theming\ImageManager;
 use OCA\Theming\Util;
 use OCP\App\IAppManager;
 use OCP\AppFramework\Http;
@@ -74,6 +75,8 @@ class ThemingControllerTest extends TestCase {
 	private $appManager;
 	/** @var IAppData|\PHPUnit_Framework_MockObject_MockObject */
 	private $appData;
+	/** @var ImageManager|\PHPUnit_Framework_MockObject_MockObject */
+	private $imageManager;
 	/** @var SCSSCacher */
 	private $scssCacher;
 	/** @var IURLGenerator */
@@ -94,6 +97,7 @@ class ThemingControllerTest extends TestCase {
 		$this->tempManager = \OC::$server->getTempManager();
 		$this->scssCacher = $this->createMock(SCSSCacher::class);
 		$this->urlGenerator = $this->createMock(IURLGenerator::class);
+		$this->imageManager = $this->createMock(ImageManager::class);
 
 		$this->themingController = new ThemingController(
 			'theming',
@@ -107,7 +111,8 @@ class ThemingControllerTest extends TestCase {
 			$this->appData,
 			$this->scssCacher,
 			$this->urlGenerator,
-			$this->appManager
+			$this->appManager,
+			$this->imageManager
 		);
 
 		return parent::setUp();
@@ -211,17 +216,12 @@ class ThemingControllerTest extends TestCase {
 		$this->request
 			->expects($this->at(0))
 			->method('getParam')
-			->with('backgroundColor')
-			->willReturn(false);
+			->with('key')
+			->willReturn('logo');
 		$this->request
 			->expects($this->at(1))
 			->method('getUploadedFile')
-			->with('uploadlogo')
-			->willReturn(null);
-		$this->request
-			->expects($this->at(2))
-			->method('getUploadedFile')
-			->with('upload-login-background')
+			->with('image')
 			->willReturn(null);
 		$this->l10n
 			->expects($this->any())
@@ -241,30 +241,25 @@ class ThemingControllerTest extends TestCase {
 			Http::STATUS_UNPROCESSABLE_ENTITY
 		);
 
-		$this->assertEquals($expected, $this->themingController->updateLogo());
+		$this->assertEquals($expected, $this->themingController->uploadImage());
 	}
 
 	public function testUpdateLogoInvalidMimeType() {
 		$this->request
 			->expects($this->at(0))
 			->method('getParam')
-			->with('backgroundColor')
-			->willReturn(false);
+			->with('key')
+			->willReturn('logo');
 		$this->request
 			->expects($this->at(1))
 			->method('getUploadedFile')
-			->with('uploadlogo')
+			->with('image')
 			->willReturn([
 				'tmp_name' => 'logo.pdf',
 				'type' => 'application/pdf',
 				'name' => 'logo.pdf',
 				'error' => 0,
 			]);
-		$this->request
-			->expects($this->at(2))
-			->method('getUploadedFile')
-			->with('upload-login-background')
-			->willReturn(null);
 		$this->l10n
 			->expects($this->any())
 			->method('t')
@@ -290,30 +285,7 @@ class ThemingControllerTest extends TestCase {
 			Http::STATUS_UNPROCESSABLE_ENTITY
 		);
 
-		$this->assertEquals($expected, $this->themingController->updateLogo());
-	}
-
-	public function testUpdateBackgroundColor() {
-		$this->request
-			->expects($this->at(0))
-			->method('getParam')
-			->with('backgroundColor')
-			->willReturn(true);
-		$this->themingDefaults
-			->expects($this->once())
-			->method('set')
-			->with('backgroundMime', 'backgroundColor');
-		$expected = new DataResponse(
-			[
-				'data' =>
-					[
-						'name' => 'backgroundColor',
-						'message' => $this->l10n->t('Saved')
-					],
-				'status' => 'success'
-			]
-		);
-		$this->assertEquals($expected, $this->themingController->updateLogo());
+		$this->assertEquals($expected, $this->themingController->uploadImage());
 	}
 
 	public function dataUpdateImages() {
@@ -336,23 +308,18 @@ class ThemingControllerTest extends TestCase {
 		$this->request
 			->expects($this->at(0))
 			->method('getParam')
-			->with('backgroundColor')
-			->willReturn(false);
+			->with('key')
+			->willReturn('logo');
 		$this->request
 			->expects($this->at(1))
 			->method('getUploadedFile')
-			->with('uploadlogo')
+			->with('image')
 			->willReturn([
 				'tmp_name' => $tmpLogo,
 				'type' => $mimeType,
 				'name' => 'logo.svg',
 				'error' => 0,
 			]);
-		$this->request
-			->expects($this->at(2))
-			->method('getUploadedFile')
-			->with('upload-login-background')
-			->willReturn(null);
 		$this->l10n
 			->expects($this->any())
 			->method('t')
@@ -385,18 +352,27 @@ class ThemingControllerTest extends TestCase {
 			->method('newFile')
 			->with('logo')
 			->willReturn($file);
+		$this->urlGenerator->expects($this->once())
+			->method('linkTo')
+			->willReturn('serverCss');
+		$this->imageManager->expects($this->once())
+			->method('getImageUrl')
+			->with('logo')
+			->willReturn('imageUrl');
 		$expected = new DataResponse(
 			[
 				'data' =>
 					[
 						'name' => 'logo.svg',
 						'message' => 'Saved',
+						'url' => 'imageUrl',
+						'serverCssUrl' => 'serverCss'
 					],
 				'status' => 'success'
 			]
 		);
 
-		$this->assertEquals($expected, $this->themingController->updateLogo());
+		$this->assertEquals($expected, $this->themingController->uploadImage());
 	}
 
 	/** @dataProvider dataUpdateImages */
@@ -408,17 +384,12 @@ class ThemingControllerTest extends TestCase {
 		$this->request
 			->expects($this->at(0))
 			->method('getParam')
-			->with('backgroundColor')
-			->willReturn(false);
+			->with('key')
+			->willReturn('background');
 		$this->request
 			->expects($this->at(1))
 			->method('getUploadedFile')
-			->with('uploadlogo')
-			->willReturn(null);
-		$this->request
-			->expects($this->at(2))
-			->method('getUploadedFile')
-			->with('upload-login-background')
+			->with('image')
 			->willReturn([
 				'tmp_name' => $tmpLogo,
 				'type' => 'text/svg',
@@ -457,17 +428,26 @@ class ThemingControllerTest extends TestCase {
 			->with('background')
 			->willReturn($file);
 
+		$this->urlGenerator->expects($this->once())
+			->method('linkTo')
+			->willReturn('serverCss');
+		$this->imageManager->expects($this->once())
+			->method('getImageUrl')
+			->with('background')
+			->willReturn('imageUrl');
 		$expected = new DataResponse(
 			[
 				'data' =>
 					[
 						'name' => 'logo.svg',
 						'message' => 'Saved',
+						'url' => 'imageUrl',
+						'serverCssUrl' => 'serverCss'
 					],
 				'status' => 'success'
 			]
 		);
-		$this->assertEquals($expected, $this->themingController->updateLogo());
+		$this->assertEquals($expected, $this->themingController->uploadImage());
 	}
 
 	public function testUpdateLogoLoginScreenUploadWithInvalidImage() {
@@ -478,20 +458,15 @@ class ThemingControllerTest extends TestCase {
 		$this->request
 			->expects($this->at(0))
 			->method('getParam')
-			->with('backgroundColor')
-			->willReturn(false);
+			->with('key')
+			->willReturn('logo');
 		$this->request
 			->expects($this->at(1))
 			->method('getUploadedFile')
-			->with('uploadlogo')
-			->willReturn(null);
-		$this->request
-			->expects($this->at(2))
-			->method('getUploadedFile')
-			->with('upload-login-background')
+			->with('image')
 			->willReturn([
 				'tmp_name' => $tmpLogo,
-				'type' => 'text/svg',
+				'type' => 'foobar',
 				'name' => 'logo.svg',
 				'error' => 0,
 			]);
@@ -519,7 +494,7 @@ class ThemingControllerTest extends TestCase {
 			],
 			Http::STATUS_UNPROCESSABLE_ENTITY
 		);
-		$this->assertEquals($expected, $this->themingController->updateLogo());
+		$this->assertEquals($expected, $this->themingController->uploadImage());
 	}
 
 	public function dataPhpUploadErrors() {
@@ -541,17 +516,12 @@ class ThemingControllerTest extends TestCase {
 		$this->request
 			->expects($this->at(0))
 			->method('getParam')
-			->with('backgroundColor')
-			->willReturn(false);
+			->with('key')
+			->willReturn('background');
 		$this->request
 			->expects($this->at(1))
 			->method('getUploadedFile')
-			->with('uploadlogo')
-			->willReturn(null);
-		$this->request
-			->expects($this->at(2))
-			->method('getUploadedFile')
-			->with('upload-login-background')
+			->with('image')
 			->willReturn([
 				'tmp_name' => '',
 				'type' => 'text/svg',
@@ -575,7 +545,7 @@ class ThemingControllerTest extends TestCase {
 			],
 			Http::STATUS_UNPROCESSABLE_ENTITY
 		);
-		$this->assertEquals($expected, $this->themingController->updateLogo());
+		$this->assertEquals($expected, $this->themingController->uploadImage());
 	}
 
 	/**
@@ -585,23 +555,18 @@ class ThemingControllerTest extends TestCase {
 		$this->request
 			->expects($this->at(0))
 			->method('getParam')
-			->with('backgroundColor')
-			->willReturn(false);
+			->with('key')
+			->willReturn('background');
 		$this->request
 			->expects($this->at(1))
 			->method('getUploadedFile')
-			->with('uploadlogo')
+			->with('image')
 			->willReturn([
 				'tmp_name' => '',
 				'type' => 'text/svg',
 				'name' => 'logo.svg',
 				'error' => $error,
 			]);
-		$this->request
-			->expects($this->at(2))
-			->method('getUploadedFile')
-			->with('upload-login-background')
-			->willReturn(null);
 		$this->l10n
 			->expects($this->any())
 			->method('t')
@@ -619,7 +584,7 @@ class ThemingControllerTest extends TestCase {
 			],
 			Http::STATUS_UNPROCESSABLE_ENTITY
 		);
-		$this->assertEquals($expected, $this->themingController->updateLogo());
+		$this->assertEquals($expected, $this->themingController->uploadImage());
 	}
 
 	public function testUndo() {
@@ -687,21 +652,9 @@ class ThemingControllerTest extends TestCase {
 			->method('linkTo')
 			->with('', '/core/css/someHash-server.scss')
 			->willReturn('/nextcloudWebroot/core/css/someHash-server.scss');
-		$folder = $this->createMock(ISimpleFolder::class);
-		$file = $this->createMock(ISimpleFile::class);
-		$this->appData
-			->expects($this->once())
-			->method('getFolder')
-			->with('images')
-			->willReturn($folder);
-		$folder
-			->expects($this->once())
-			->method('getFile')
-			->with($filename)
-			->willReturn($file);
-		$file
-			->expects($this->once())
-			->method('delete');
+		$this->imageManager->expects($this->once())
+			->method('delete')
+			->with($filename);
 
 		$expected = new DataResponse(
 			[
@@ -720,27 +673,19 @@ class ThemingControllerTest extends TestCase {
 
 
 	public function testGetLogoNotExistent() {
-		$this->appData->method('getFolder')
-			->with($this->equalTo('images'))
+		$this->imageManager->method('getImage')
+			->with($this->equalTo('logo'))
 			->willThrowException(new NotFoundException());
 
 		$expected = new Http\NotFoundResponse();
-		$this->assertEquals($expected, $this->themingController->getLogo());
+		$this->assertEquals($expected, $this->themingController->getImage('logo'));
 	}
 
 	public function testGetLogo() {
 		$file = $this->createMock(ISimpleFile::class);
-		$folder = $this->createMock(ISimpleFolder::class);
-		$this->appData
-			->expects($this->once())
-			->method('getFolder')
-			->with('images')
-			->willReturn($folder);
-		$folder->expects($this->once())
-			->method('getFile')
-			->with('logo')
+		$this->imageManager->expects($this->once())
+			->method('getImage')
 			->willReturn($file);
-
 		$this->config
 			->expects($this->once())
 			->method('getAppValue')
@@ -755,29 +700,22 @@ class ThemingControllerTest extends TestCase {
 		$expected->addHeader('Expires', $expires->format(\DateTime::RFC2822));
 		$expected->addHeader('Pragma', 'cache');
 		$expected->addHeader('Content-Type', 'text/svg');
-		@$this->assertEquals($expected, $this->themingController->getLogo());
+		@$this->assertEquals($expected, $this->themingController->getImage('logo'));
 	}
 
 
 	public function testGetLoginBackgroundNotExistent() {
-		$this->appData->method('getFolder')
-			->with($this->equalTo('images'))
+		$this->imageManager->method('getImage')
+			->with($this->equalTo('background'))
 			->willThrowException(new NotFoundException());
 		$expected = new Http\NotFoundResponse();
-		$this->assertEquals($expected, $this->themingController->getLoginBackground());
+		$this->assertEquals($expected, $this->themingController->getImage('background'));
 	}
 
 	public function testGetLoginBackground() {
 		$file = $this->createMock(ISimpleFile::class);
-		$folder = $this->createMock(ISimpleFolder::class);
-		$this->appData
-			->expects($this->once())
-			->method('getFolder')
-			->with('images')
-			->willReturn($folder);
-		$folder->expects($this->once())
-			->method('getFile')
-			->with('background')
+		$this->imageManager->expects($this->once())
+			->method('getImage')
 			->willReturn($file);
 
 		$this->config
@@ -794,7 +732,7 @@ class ThemingControllerTest extends TestCase {
 		$expected->addHeader('Expires', $expires->format(\DateTime::RFC2822));
 		$expected->addHeader('Pragma', 'cache');
 		$expected->addHeader('Content-Type', 'image/png');
-		@$this->assertEquals($expected, $this->themingController->getLoginBackground());
+		@$this->assertEquals($expected, $this->themingController->getImage('background'));
 	}
 
 

apps/theming/tests/ImageManagerTest.php

@@ -23,14 +23,16 @@
  */
 namespace OCA\Theming\Tests;
 
+use OCA\Theming\ImageManager;
 use OCP\Files\SimpleFS\ISimpleFile;
 use OCP\IConfig;
+use OCP\IURLGenerator;
 use Test\TestCase;
 use OCP\Files\SimpleFS\ISimpleFolder;
 use OCP\Files\IAppData;
 use OCP\Files\NotFoundException;
 
-class ImageManager extends TestCase {
+class ImageManagerTest extends TestCase {
 
 	/** @var IConfig|\PHPUnit_Framework_MockObject_MockObject */
 	protected $config;
@@ -38,14 +40,18 @@ class ImageManager extends TestCase {
 	protected $appData;
 	/** @var ImageManager */
 	protected $imageManager;
+	/** @var IURLGenerator|\PHPUnit_Framework_MockObject_MockObject */
+	private $urlGenerator;
 
 	protected function setUp() {
 		parent::setUp();
-		$this->config = $this->getMockBuilder(IConfig::class)->getMock();
-		$this->appData = $this->getMockBuilder('OCP\Files\IAppData')->getMock();
-		$this->imageManager = new \OCA\Theming\ImageManager(
+		$this->config = $this->createMock(IConfig::class);
+		$this->appData = $this->createMock(IAppData::class);
+		$this->urlGenerator = $this->createMock(IURLGenerator::class);
+		$this->imageManager = new ImageManager(
 			$this->config,
-			$this->appData
+			$this->appData,
+			$this->urlGenerator
 		);
 	}
 
@@ -85,12 +91,12 @@ class ImageManager extends TestCase {
 	}
 
 	public function testGetCachedImage() {
+		$expected = $this->createMock(ISimpleFile::class);
 		$folder = $this->setupCacheFolder();
 		$folder->expects($this->once())
 			->method('getFile')
 			->with('filename')
-			->willReturn('filecontent');
-		$expected = 'filecontent';
+			->willReturn($expected);
 		$this->assertEquals($expected, $this->imageManager->getCachedImage('filename'));
 	}
 

apps/theming/tests/Settings/AdminTest.php

@@ -27,6 +27,7 @@
 
 namespace OCA\Theming\Tests\Settings;
 
+use OCA\Theming\ImageManager;
 use OCA\Theming\Settings\Admin;
 use OCA\Theming\ThemingDefaults;
 use OCP\AppFramework\Http\TemplateResponse;
@@ -44,21 +45,25 @@ class AdminTest extends TestCase {
 	private $themingDefaults;
 	/** @var IURLGenerator */
 	private $urlGenerator;
+	/** @var ImageManager */
+	private $imageManager;
 	/** @var IL10N */
 	private $l10n;
 
 	public function setUp() {
 		parent::setUp();
-		$this->config = $this->getMockBuilder(IConfig::class)->getMock();
-		$this->l10n = $this->getMockBuilder(IL10N::class)->getMock();
-		$this->themingDefaults = $this->getMockBuilder('\OCA\Theming\ThemingDefaults')->disableOriginalConstructor()->getMock();
-		$this->urlGenerator = $this->getMockBuilder(IURLGenerator::class)->getMock();
+		$this->config = $this->createMock(IConfig::class);
+		$this->l10n = $this->createMock(IL10N::class);
+		$this->themingDefaults = $this->createMock(ThemingDefaults::class);
+		$this->urlGenerator = $this->createMock(IURLGenerator::class);
+		$this->imageManager = $this->createMock(ImageManager::class);
 
 		$this->admin = new Admin(
 			$this->config,
 			$this->l10n,
 			$this->themingDefaults,
-			$this->urlGenerator
+			$this->urlGenerator,
+			$this->imageManager
 		);
 	}
 
@@ -87,7 +92,7 @@ class AdminTest extends TestCase {
 		$this->urlGenerator
 			->expects($this->once())
 			->method('linkToRoute')
-			->with('theming.Theming.updateLogo')
+			->with('theming.Theming.uploadImage')
 			->willReturn('/my/route');
 		$params = [
 			'themable' => true,
@@ -97,12 +102,9 @@ class AdminTest extends TestCase {
 			'slogan' => 'MySlogan',
 			'color' => '#fff',
 			'uploadLogoRoute' => '/my/route',
-			'logo' => null,
-			'logoMime' => null,
-			'background' => null,
-			'backgroundMime' => null,
 			'canThemeIcons' => null,
 			'iconDocs' => null,
+			'images' => [],
 		];
 
 		$expected = new TemplateResponse('theming', 'settings-admin', $params, '');
@@ -139,7 +141,7 @@ class AdminTest extends TestCase {
 		$this->urlGenerator
 			->expects($this->once())
 			->method('linkToRoute')
-			->with('theming.Theming.updateLogo')
+			->with('theming.Theming.uploadImage')
 			->willReturn('/my/route');
 		$params = [
 			'themable' => false,
@@ -149,12 +151,9 @@ class AdminTest extends TestCase {
 			'slogan' => 'MySlogan',
 			'color' => '#fff',
 			'uploadLogoRoute' => '/my/route',
-			'logo' => null,
-			'logoMime' => null,
-			'background' => null,
-			'backgroundMime' => null,
 			'canThemeIcons' => null,
-			'iconDocs' => null,
+			'iconDocs' => '',
+			'images' => [],
 		];
 
 		$expected = new TemplateResponse('theming', 'settings-admin', $params, '');

apps/theming/tests/ThemingDefaultsTest.php

@@ -29,6 +29,7 @@
  */
 namespace OCA\Theming\Tests;
 
+use OCA\Theming\ImageManager;
 use OCA\Theming\ThemingDefaults;
 use OCP\App\IAppManager;
 use OCP\Files\IAppData;
@@ -64,30 +65,31 @@ class ThemingDefaultsTest extends TestCase {
 	private $cache;
 	/** @var IAppManager|\PHPUnit_Framework_MockObject_MockObject */
 	private $appManager;
+	/** @var ImageManager|\PHPUnit_Framework_MockObject_MockObject */
+	private $imageManager;
 
 	public function setUp() {
 		parent::setUp();
 		$this->config = $this->createMock(IConfig::class);
 		$this->l10n = $this->createMock(IL10N::class);
 		$this->urlGenerator = $this->createMock(IURLGenerator::class);
-		$this->appData = $this->createMock(IAppData::class);
 		$this->cacheFactory = $this->createMock(ICacheFactory::class);
 		$this->cache = $this->createMock(ICache::class);
 		$this->util = $this->createMock(Util::class);
+		$this->imageManager = $this->createMock(ImageManager::class);
 		$this->appManager = $this->createMock(IAppManager::class);
 		$this->defaults = new \OC_Defaults();
-		$this->cacheFactory
+		$this->urlGenerator
 			->expects($this->any())
-			->method('createDistributed')
-			->with('theming-')
-			->willReturn($this->cache);
+			->method('getBaseUrl')
+			->willReturn('');
 		$this->template = new ThemingDefaults(
 			$this->config,
 			$this->l10n,
 			$this->urlGenerator,
-			$this->appData,
 			$this->cacheFactory,
 			$this->util,
+			$this->imageManager,
 			$this->appManager
 		);
 	}
@@ -273,8 +275,18 @@ class ThemingDefaultsTest extends TestCase {
 			->expects($this->at(2))
 			->method('setAppValue')
 			->with('theming', 'cachebuster', 16);
+		$this->cacheFactory
+			->expects($this->at(0))
+			->method('createDistributed')
+			->with('theming-')
+			->willReturn($this->cache);
+		$this->cacheFactory
+			->expects($this->at(1))
+			->method('createDistributed')
+			->with('imagePath')
+			->willReturn($this->cache);
 		$this->cache
-			->expects($this->once())
+			->expects($this->any())
 			->method('clear')
 			->with('');
 		$this->template->set('MySetting', 'MyValue');
@@ -390,41 +402,19 @@ class ThemingDefaultsTest extends TestCase {
 		$this->assertSame('', $this->template->undo('defaultitem'));
 	}
 
-	public function testGetBackgroundDefault() {
-		$this->config
+	public function testGetBackground() {
+		$this->imageManager
 			->expects($this->once())
-			->method('getAppValue')
-			->with('theming', 'cachebuster', '0')
-			->willReturn('0');
-		$this->util->expects($this->once())
-			->method('isBackgroundThemed')
-			->willReturn(false);
-		$this->urlGenerator->expects($this->once())
-			->method('imagePath')
-			->with('core', 'background.png')
-			->willReturn('core-background');
-		$this->assertEquals('core-background?v=0', $this->template->getBackground());
-	}
-
-	public function testGetBackgroundCustom() {
-		$this->config
-			->expects($this->once())
-			->method('getAppValue')
-			->with('theming', 'cachebuster', '0')
-			->willReturn('0');
-		$this->util->expects($this->once())
-			->method('isBackgroundThemed')
-			->willReturn(true);
-		$this->urlGenerator->expects($this->once())
-			->method('linkToRoute')
-			->with('theming.Theming.getLoginBackground')
-			->willReturn('custom-background');
+			->method('getImageUrl')
+			->with('background')
+			->willReturn('custom-background?v=0');
 		$this->assertEquals('custom-background?v=0', $this->template->getBackground());
 	}
 
 	private function getLogoHelper($withName, $useSvg) {
-		$this->appData->expects($this->once())
-			->method('getFolder')
+		$this->imageManager->expects($this->any())
+			->method('getImage')
+			->with('logo')
 			->willThrowException(new NotFoundException());
 		$this->config
 			->expects($this->at(0))
@@ -436,11 +426,6 @@ class ThemingDefaultsTest extends TestCase {
 			->method('getAppValue')
 			->with('theming', 'cachebuster', '0')
 			->willReturn('0');
-		$this->appData
-			->expects($this->once())
-			->method('getFolder')
-			->with('images')
-			->willThrowException(new \Exception());
 		$this->urlGenerator->expects($this->once())
 			->method('imagePath')
 			->with('core', $withName)
@@ -457,14 +442,11 @@ class ThemingDefaultsTest extends TestCase {
 	}
 
 	public function testGetLogoCustom() {
-		$folder = $this->createMock(ISimpleFolder::class);
 		$file = $this->createMock(ISimpleFile::class);
-		$folder->expects($this->once())
-			->method('getFile')
+		$this->imageManager->expects($this->once())
+			->method('getImage')
+			->with('logo')
 			->willReturn($file);
-		$this->appData->expects($this->once())
-			->method('getFolder')
-			->willReturn($folder);
 		$this->config
 			->expects($this->at(0))
 			->method('getAppValue')
@@ -477,12 +459,16 @@ class ThemingDefaultsTest extends TestCase {
 			->willReturn('0');
 		$this->urlGenerator->expects($this->once())
 			->method('linkToRoute')
-			->with('theming.Theming.getLogo')
+			->with('theming.Theming.getImage')
 			->willReturn('custom-logo');
 		$this->assertEquals('custom-logo' . '?v=0', $this->template->getLogo());
 	}
 
 	public function testGetScssVariablesCached() {
+		$this->cacheFactory->expects($this->once())
+			->method('createDistributed')
+			->with('theming-')
+			->willReturn($this->cache);
 		$this->cache->expects($this->once())->method('get')->with('getScssVariables')->willReturn(['foo'=>'bar']);
 		$this->assertEquals(['foo'=>'bar'], $this->template->getScssVariables());
 	}
@@ -491,31 +477,25 @@ class ThemingDefaultsTest extends TestCase {
 		$this->config->expects($this->at(0))->method('getAppValue')->with('theming', 'cachebuster', '0')->willReturn('0');
 		$this->config->expects($this->at(1))->method('getAppValue')->with('theming', 'logoMime', false)->willReturn('jpeg');
 		$this->config->expects($this->at(2))->method('getAppValue')->with('theming', 'backgroundMime', false)->willReturn('jpeg');
-		$this->config->expects($this->at(3))->method('getAppValue')->with('theming', 'logoMime', false)->willReturn('jpeg');
-		$this->config->expects($this->at(4))->method('getAppValue')->with('theming', 'cachebuster', '0')->willReturn('0');
-		$this->util->expects($this->once())->method('isBackgroundThemed')->willReturn(true);
-		$this->config->expects($this->at(5))->method('getAppValue')->with('theming', 'cachebuster', '0')->willReturn('0');
-		$this->config->expects($this->at(6))->method('getAppValue')->with('theming', 'color', null)->willReturn($this->defaults->getColorPrimary());
+		$this->config->expects($this->at(3))->method('getAppValue')->with('theming', 'logoheaderMime', false)->willReturn('jpeg');
+		$this->config->expects($this->at(4))->method('getAppValue')->with('theming', 'faviconMime', false)->willReturn('jpeg');
+
+		$this->config->expects($this->at(5))->method('getAppValue')->with('theming', 'color', null)->willReturn($this->defaults->getColorPrimary());
+		$this->config->expects($this->at(6))->method('getAppValue')->with('theming', 'color', $this->defaults->getColorPrimary())->willReturn($this->defaults->getColorPrimary());
 		$this->config->expects($this->at(7))->method('getAppValue')->with('theming', 'color', $this->defaults->getColorPrimary())->willReturn($this->defaults->getColorPrimary());
 		$this->config->expects($this->at(8))->method('getAppValue')->with('theming', 'color', $this->defaults->getColorPrimary())->willReturn($this->defaults->getColorPrimary());
-		$this->config->expects($this->at(9))->method('getAppValue')->with('theming', 'color', $this->defaults->getColorPrimary())->willReturn($this->defaults->getColorPrimary());
 
 		$this->util->expects($this->any())->method('invertTextColor')->with($this->defaults->getColorPrimary())->willReturn(false);
 		$this->util->expects($this->any())->method('elementColor')->with($this->defaults->getColorPrimary())->willReturn('#aaaaaa');
+		$this->cacheFactory->expects($this->once())
+			->method('createDistributed')
+			->with('theming-')
+			->willReturn($this->cache);
 		$this->cache->expects($this->once())->method('get')->with('getScssVariables')->willReturn(null);
-		$folder = $this->createMock(ISimpleFolder::class);
-		$file = $this->createMock(ISimpleFile::class);
-		$folder->expects($this->any())->method('getFile')->willReturn($file);
-		$this->appData->expects($this->any())
-			->method('getFolder')
-			->willReturn($folder);
-
-		$this->urlGenerator->expects($this->exactly(2))
-			->method('linkToRoute')
-			->willReturnMap([
-				['theming.Theming.getLogo', [], 'custom-logo'],
-				['theming.Theming.getLoginBackground', [], 'custom-background'],
-			]);
+		$this->imageManager->expects($this->at(0))->method('getImageUrl')->with('logo')->willReturn('custom-logo?v=0');
+		$this->imageManager->expects($this->at(1))->method('getImageUrl')->with('logoheader')->willReturn('custom-logoheader?v=0');
+		$this->imageManager->expects($this->at(2))->method('getImageUrl')->with('favicon')->willReturn('custom-favicon?v=0');
+		$this->imageManager->expects($this->at(3))->method('getImageUrl')->with('background')->willReturn('custom-background?v=0');
 
 		$expected = [
 			'theming-cachebuster' => '\'0\'',
@@ -526,8 +506,11 @@ class ThemingDefaultsTest extends TestCase {
 			'color-primary' => $this->defaults->getColorPrimary(),
 			'color-primary-text' => '#ffffff',
 			'image-login-plain' => 'false',
-			'color-primary-element' => '#aaaaaa'
-
+			'color-primary-element' => '#aaaaaa',
+			'theming-logoheader-mime' => '\'jpeg\'',
+			'theming-favicon-mime' => '\'jpeg\'',
+			'image-logoheader' => '\'custom-logoheader?v=0\'',
+			'image-favicon' => '\'custom-favicon?v=0\''
 		];
 		$this->assertEquals($expected, $this->template->getScssVariables());
 	}