From 6a9d5d60dc13bd43604cf30b3d9fe90364038c45 Mon Sep 17 00:00:00 2001 From: Julius Haertl Date: Thu, 9 Feb 2017 10:55:28 +0100 Subject: [PATCH 1/7] Move theming images to AppData Signed-off-by: Julius Haertl --- .../lib/Controller/ThemingController.php | 60 +++++++++++-------- apps/theming/lib/ImageManager.php | 2 +- 2 files changed, 36 insertions(+), 26 deletions(-) diff --git a/apps/theming/lib/Controller/ThemingController.php b/apps/theming/lib/Controller/ThemingController.php index 24ac1c7d8d5..19999d1362d 100644 --- a/apps/theming/lib/Controller/ThemingController.php +++ b/apps/theming/lib/Controller/ThemingController.php @@ -33,10 +33,10 @@ use OCP\AppFramework\Http; use OCP\AppFramework\Http\DataDownloadResponse; use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\Http\NotFoundResponse; -use OCP\AppFramework\Http\StreamResponse; +use OCP\AppFramework\Http\FileDisplayResponse; use OCP\AppFramework\Utility\ITimeFactory; use OCP\Files\File; -use OCP\Files\IRootFolder; +use OCP\Files\IAppData; use OCP\Files\NotFoundException; use OCP\IConfig; use OCP\IL10N; @@ -62,10 +62,10 @@ class ThemingController extends Controller { private $l; /** @var IConfig */ private $config; - /** @var IRootFolder */ - private $rootFolder; /** @var ITempManager */ private $tempManager; + /** @var IAppData */ + private $appData; /** * ThemingController constructor. @@ -77,8 +77,8 @@ class ThemingController extends Controller { * @param Util $util * @param ITimeFactory $timeFactory * @param IL10N $l - * @param IRootFolder $rootFolder * @param ITempManager $tempManager + * @param IAppData $appData */ public function __construct( $appName, @@ -88,8 +88,8 @@ class ThemingController extends Controller { Util $util, ITimeFactory $timeFactory, IL10N $l, - IRootFolder $rootFolder, - ITempManager $tempManager + ITempManager $tempManager, + IAppData $appData ) { parent::__construct($appName, $request); @@ -98,8 +98,8 @@ class ThemingController extends Controller { $this->timeFactory = $timeFactory; $this->l = $l; $this->config = $config; - $this->rootFolder = $rootFolder; $this->tempManager = $tempManager; + $this->appData = $appData; } /** @@ -183,16 +183,22 @@ class ThemingController extends Controller { Http::STATUS_UNPROCESSABLE_ENTITY ); } + $name = ''; + try { + $folder = $this->appData->getFolder('images'); + } catch (NotFoundException $e) { + $folder = $this->appData->newFolder('images'); + } + if(!empty($newLogo)) { - $target = $this->rootFolder->newFile('themedinstancelogo'); - stream_copy_to_stream(fopen($newLogo['tmp_name'], 'r'), $target->fopen('w')); + $target = $folder->newFile('logo'); + $target->putContent(file_get_contents($newLogo['tmp_name'], 'r')); $this->template->set('logoMime', $newLogo['type']); $name = $newLogo['name']; } if(!empty($newBackgroundLogo)) { - $target = $this->rootFolder->newFile('themedbackgroundlogo'); - + $target = $folder->newFile('background'); $image = @imagecreatefromstring(file_get_contents($newBackgroundLogo['tmp_name'], 'r')); if($image === false) { return new DataResponse( @@ -219,7 +225,7 @@ class ThemingController extends Controller { imagejpeg($image, $tmpFile, 75); imagedestroy($image); - stream_copy_to_stream(fopen($tmpFile, 'r'), $target->fopen('w')); + $target->putContent(file_get_contents($tmpFile, 'r')); $this->template->set('backgroundMime', $newBackgroundLogo['type']); $name = $newBackgroundLogo['name']; } @@ -260,22 +266,24 @@ class ThemingController extends Controller { * @PublicPage * @NoCSRFRequired * - * @return StreamResponse|NotFoundResponse + * @return Http\FileDisplayResponse|NotFoundResponse */ public function getLogo() { try { /** @var File $file */ - $file = $this->rootFolder->get('themedinstancelogo'); + $file = $this->appData->getFolder('images')->getFile('logo'); } catch (NotFoundException $e) { return new NotFoundResponse(); } - $response = new Http\StreamResponse($file->fopen('r')); + $response = new FileDisplayResponse($file); $response->cacheFor(3600); - $response->addHeader('Expires', date(\DateTime::RFC2822, $this->timeFactory->getTime())); - $response->addHeader('Content-Disposition', 'attachment'); - $response->addHeader('Content-Type', $this->config->getAppValue($this->appName, 'logoMime', '')); + $expires = new \DateTime(); + $expires->setTimestamp($this->timeFactory->getTime()); + $expires->add(new \DateInterval('PT24H')); + $response->addHeader('Expires', $expires->format(\DateTime::RFC2822)); $response->addHeader('Pragma', 'cache'); + $response->addHeader('Content-Type', $this->config->getAppValue($this->appName, 'logoMime', '')); return $response; } @@ -283,22 +291,24 @@ class ThemingController extends Controller { * @PublicPage * @NoCSRFRequired * - * @return StreamResponse|NotFoundResponse + * @return FileDisplayResponse|NotFoundResponse */ public function getLoginBackground() { try { /** @var File $file */ - $file = $this->rootFolder->get('themedbackgroundlogo'); + $file = $this->appData->getFolder('images')->getFile('background'); } catch (NotFoundException $e) { return new NotFoundResponse(); } - $response = new StreamResponse($file->fopen('r')); + $response = new FileDisplayResponse($file); $response->cacheFor(3600); - $response->addHeader('Expires', date(\DateTime::RFC2822, $this->timeFactory->getTime())); - $response->addHeader('Content-Disposition', 'attachment'); - $response->addHeader('Content-Type', $this->config->getAppValue($this->appName, 'backgroundMime', '')); + $expires = new \DateTime(); + $expires->setTimestamp($this->timeFactory->getTime()); + $expires->add(new \DateInterval('PT24H')); + $response->addHeader('Expires', $expires->format(\DateTime::RFC2822)); $response->addHeader('Pragma', 'cache'); + $response->addHeader('Content-Type', $this->config->getAppValue($this->appName, 'backgroundMime', '')); return $response; } diff --git a/apps/theming/lib/ImageManager.php b/apps/theming/lib/ImageManager.php index 4cd43e02054..88e456a2969 100644 --- a/apps/theming/lib/ImageManager.php +++ b/apps/theming/lib/ImageManager.php @@ -104,7 +104,7 @@ class ImageManager { $currentFolder = $this->getCacheFolder(); $folders = $this->appData->getDirectoryListing(); foreach ($folders as $folder) { - if ($folder->getName() !== $currentFolder->getName()) { + if ($folder->getName() !== 'images' && $folder->getName() !== $currentFolder->getName()) { $folder->delete(); } } From 60d77ceccb88414cfa426f237549f3ac9684f7b3 Mon Sep 17 00:00:00 2001 From: Julius Haertl Date: Thu, 9 Feb 2017 10:56:42 +0100 Subject: [PATCH 2/7] Add repair step to move existing theming images Signed-off-by: Julius Haertl --- apps/theming/appinfo/info.xml | 8 +- apps/theming/lib/Migration/ThemingImages.php | 78 ++++++++++++++++++++ 2 files changed, 85 insertions(+), 1 deletion(-) create mode 100644 apps/theming/lib/Migration/ThemingImages.php diff --git a/apps/theming/appinfo/info.xml b/apps/theming/appinfo/info.xml index 9ded5c89b87..13a0f161a47 100644 --- a/apps/theming/appinfo/info.xml +++ b/apps/theming/appinfo/info.xml @@ -5,7 +5,7 @@ Adjust the Nextcloud theme AGPL Nextcloud - 1.2.0 + 1.3.0 Theming other @@ -23,4 +23,10 @@ OCA\Theming\Settings\Admin OCA\Theming\Settings\Section + + + + OCA\Theming\Migration\ThemingImages + + diff --git a/apps/theming/lib/Migration/ThemingImages.php b/apps/theming/lib/Migration/ThemingImages.php new file mode 100644 index 00000000000..9627697f666 --- /dev/null +++ b/apps/theming/lib/Migration/ThemingImages.php @@ -0,0 +1,78 @@ + + * + * @author Julius Härtl + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + + +namespace OCA\Theming\Migration; + +use OCA\Theming\ThemingDefaults; +use OCP\Files\IAppData; +use OCP\Files\IRootFolder; +use OCP\Migration\IRepairStep; +use OCP\Migration\IOutput; +use OC\Files\Node\File; +use OCP\Files\NotFoundException; + +class ThemingImages implements IRepairStep { + + private $appData; + private $rootFolder; + + public function __construct(IAppData $appData, ThemingDefaults $defaults, IRootFolder $rootFolder) { + $this->appData = $appData; + $this->rootFolder = $rootFolder; + } + + /* + * @inheritdoc + */ + public function getName() { + return 'Move theming files to AppData storage'; + } + + /** + * @inheritdoc + */ + public function run(IOutput $output) { + $folder = $this->appData->newFolder("images"); + /** @var File $file */ + $file = null; + try { + $file = $this->rootFolder->get('themedinstancelogo'); + $logo = $folder->newFile("logo"); + $logo->putContent($file->getContent()); + $file->delete(); + } catch (NotFoundException $e) { + $output->info("No theming logo image to migrate"); + } + + try { + $file = $this->rootFolder->get('themedbackgroundlogo'); + $background = $folder->newFile("background"); + $background->putContent($file->getContent()); + $file->delete(); + } catch (NotFoundException $e) { + $output->info("No theming background image to migrate"); + } + + } +} \ No newline at end of file From 345d1f12914f2635741886b9e4b990bbbbdd2973 Mon Sep 17 00:00:00 2001 From: Julius Haertl Date: Thu, 9 Feb 2017 11:38:45 +0100 Subject: [PATCH 3/7] Adapt ThemingController tests Signed-off-by: Julius Haertl --- .../Controller/ThemingControllerTest.php | 133 +++++++++++------- 1 file changed, 81 insertions(+), 52 deletions(-) diff --git a/apps/theming/tests/Controller/ThemingControllerTest.php b/apps/theming/tests/Controller/ThemingControllerTest.php index 97a5e985860..97770e7ce62 100644 --- a/apps/theming/tests/Controller/ThemingControllerTest.php +++ b/apps/theming/tests/Controller/ThemingControllerTest.php @@ -29,7 +29,7 @@ use OCA\Theming\Util; use OCP\App\IAppManager; use OCP\AppFramework\Http; use OCP\AppFramework\Http\DataResponse; -use OCP\Files\File; +use OCP\Files\IAppData; use OCP\Files\IRootFolder; use OCP\Files\NotFoundException; use OCP\IConfig; @@ -58,8 +58,10 @@ class ThemingControllerTest extends TestCase { private $rootFolder; /** @var ITempManager */ private $tempManager; - /** @var IAppManager */ + /** @var IAppManager|\PHPUnit_Framework_MockObject_MockObject */ private $appManager; + /** @var IAppData|\PHPUnit_Framework_MockObject_MockObject */ + private $appData; public function setUp() { $this->request = $this->getMockBuilder('OCP\IRequest')->getMock(); @@ -77,6 +79,7 @@ class ThemingControllerTest extends TestCase { ->method('getTime') ->willReturn(123); $this->tempManager = \OC::$server->getTempManager(); + $this->appData = $this->getMockBuilder('OCP\Files\IAppData')->getMock(); $this->themingController = new ThemingController( 'theming', @@ -86,8 +89,8 @@ class ThemingControllerTest extends TestCase { $this->util, $this->timeFactory, $this->l10n, - $this->rootFolder, - $this->tempManager + $this->tempManager, + $this->appData ); return parent::setUp(); @@ -191,19 +194,23 @@ class ThemingControllerTest extends TestCase { ->method('t') ->with('Saved') ->willReturn('Saved'); - $file = $this->getMockBuilder('\\OCP\\Files\\File') + + + $file = $this->getMockBuilder('OCP\Files\SimpleFS\ISimpleFile') ->disableOriginalConstructor() ->getMock(); - $this->rootFolder + $folder = $this->getMockBuilder('OCP\Files\SimpleFS\ISimpleFolder') + ->disableOriginalConstructor() + ->getMock(); + $this->appData ->expects($this->once()) + ->method('getFolder') + ->with('images') + ->willReturn($folder); + $folder->expects($this->once()) ->method('newFile') - ->with('themedinstancelogo') + ->with('logo') ->willReturn($file); - $file - ->expects($this->once()) - ->method('fopen') - ->with('w') - ->willReturn(fopen($destination . '/themedinstancelogo', 'w')); $expected = new DataResponse( [ @@ -221,7 +228,6 @@ class ThemingControllerTest extends TestCase { public function testUpdateLogoLoginScreenUpload() { $tmpLogo = \OC::$server->getTempManager()->getTemporaryFolder() . '/logo.svg'; - $destination = \OC::$server->getTempManager()->getTemporaryFolder(); touch($tmpLogo); file_put_contents($tmpLogo, file_get_contents(__DIR__ . '/../../../../tests/data/desktopapp.png')); @@ -244,20 +250,22 @@ class ThemingControllerTest extends TestCase { ->method('t') ->with('Saved') ->willReturn('Saved'); - $file = $this->getMockBuilder('\\OCP\\Files\\File') + + $file = $this->getMockBuilder('OCP\Files\SimpleFS\ISimpleFile') ->disableOriginalConstructor() ->getMock(); - $this->rootFolder + $folder = $this->getMockBuilder('OCP\Files\SimpleFS\ISimpleFolder') + ->disableOriginalConstructor() + ->getMock(); + $this->appData ->expects($this->once()) + ->method('getFolder') + ->with('images') + ->willReturn($folder); + $folder->expects($this->once()) ->method('newFile') - ->with('themedbackgroundlogo') + ->with('background') ->willReturn($file); - $file - ->expects($this->once()) - ->method('fopen') - ->with('w') - ->willReturn(fopen($destination . '/themedbackgroundlogo', 'w')); - $expected = new DataResponse( [ @@ -274,7 +282,6 @@ class ThemingControllerTest extends TestCase { public function testUpdateLogoLoginScreenUploadWithInvalidImage() { $tmpLogo = \OC::$server->getTempManager()->getTemporaryFolder() . '/logo.svg'; - $destination = \OC::$server->getTempManager()->getTemporaryFolder(); touch($tmpLogo); file_put_contents($tmpLogo, file_get_contents(__DIR__ . '/../../../../tests/data/data.zip')); @@ -297,14 +304,16 @@ class ThemingControllerTest extends TestCase { ->method('t') ->with('Unsupported image type') ->willReturn('Unsupported image type'); - $file = $this->getMockBuilder('\\OCP\\Files\\File') + + $folder = $this->getMockBuilder('OCP\Files\SimpleFS\ISimpleFolder') ->disableOriginalConstructor() ->getMock(); - $this->rootFolder + $this->appData ->expects($this->once()) - ->method('newFile') - ->with('themedbackgroundlogo') - ->willReturn($file); + ->method('getFolder') + ->with('images') + ->willReturn($folder); + $expected = new DataResponse( [ 'data' => @@ -344,8 +353,8 @@ class ThemingControllerTest extends TestCase { } public function testGetLogoNotExistent() { - $this->rootFolder->method('get') - ->with($this->equalTo('themedinstancelogo')) + $this->appData->method('getFolder') + ->with($this->equalTo('images')) ->willThrowException(new NotFoundException()); $expected = new Http\NotFoundResponse(); @@ -353,13 +362,21 @@ class ThemingControllerTest extends TestCase { } public function testGetLogo() { - $file = $this->createMock(File::class); - $this->rootFolder->method('get') - ->with('themedinstancelogo') + $file = $this->getMockBuilder('OCP\Files\SimpleFS\ISimpleFile') + ->disableOriginalConstructor() + ->getMock(); + $folder = $this->getMockBuilder('OCP\Files\SimpleFS\ISimpleFolder') + ->disableOriginalConstructor() + ->getMock(); + $this->appData + ->expects($this->once()) + ->method('getFolder') + ->with('images') + ->willReturn($folder); + $folder->expects($this->once()) + ->method('getFile') + ->with('logo') ->willReturn($file); - $file->method('fopen') - ->with('r') - ->willReturn('mypath'); $this->config ->expects($this->once()) @@ -367,32 +384,42 @@ class ThemingControllerTest extends TestCase { ->with('theming', 'logoMime', '') ->willReturn('text/svg'); - @$expected = new Http\StreamResponse('mypath'); + @$expected = new Http\FileDisplayResponse($file); $expected->cacheFor(3600); - $expected->addHeader('Expires', date(\DateTime::RFC2822, 123)); - $expected->addHeader('Content-Disposition', 'attachment'); - $expected->addHeader('Content-Type', 'text/svg'); + $expires = new \DateTime(); + $expires->setTimestamp($this->timeFactory->getTime()); + $expires->add(new \DateInterval('PT24H')); + $expected->addHeader('Expires', $expires->format(\DateTime::RFC2822)); $expected->addHeader('Pragma', 'cache'); + $expected->addHeader('Content-Type', 'text/svg'); @$this->assertEquals($expected, $this->themingController->getLogo()); } public function testGetLoginBackgroundNotExistent() { - $this->rootFolder->method('get') - ->with('themedbackgroundlogo') + $this->appData->method('getFolder') + ->with($this->equalTo('images')) ->willThrowException(new NotFoundException()); $expected = new Http\NotFoundResponse(); $this->assertEquals($expected, $this->themingController->getLoginBackground()); } public function testGetLoginBackground() { - $file = $this->createMock(File::class); - $this->rootFolder->method('get') - ->with('themedbackgroundlogo') + $file = $this->getMockBuilder('OCP\Files\SimpleFS\ISimpleFile') + ->disableOriginalConstructor() + ->getMock(); + $folder = $this->getMockBuilder('OCP\Files\SimpleFS\ISimpleFolder') + ->disableOriginalConstructor() + ->getMock(); + $this->appData + ->expects($this->once()) + ->method('getFolder') + ->with('images') + ->willReturn($folder); + $folder->expects($this->once()) + ->method('getFile') + ->with('background') ->willReturn($file); - $file->method('fopen') - ->with('r') - ->willReturn('mypath'); $this->config ->expects($this->once()) @@ -400,12 +427,14 @@ class ThemingControllerTest extends TestCase { ->with('theming', 'backgroundMime', '') ->willReturn('image/png'); - @$expected = new Http\StreamResponse('mypath'); + @$expected = new Http\FileDisplayResponse($file); $expected->cacheFor(3600); - $expected->addHeader('Expires', date(\DateTime::RFC2822, 123)); - $expected->addHeader('Content-Disposition', 'attachment'); - $expected->addHeader('Content-Type', 'image/png'); + $expires = new \DateTime(); + $expires->setTimestamp($this->timeFactory->getTime()); + $expires->add(new \DateInterval('PT24H')); + $expected->addHeader('Expires', $expires->format(\DateTime::RFC2822)); $expected->addHeader('Pragma', 'cache'); + $expected->addHeader('Content-Type', 'image/png'); @$this->assertEquals($expected, $this->themingController->getLoginBackground()); } From 88a0ef7d4a49403c0b803cd8716ae95688490b59 Mon Sep 17 00:00:00 2001 From: Julius Haertl Date: Thu, 9 Feb 2017 12:08:42 +0100 Subject: [PATCH 4/7] Add tests for theming migration step Signed-off-by: Julius Haertl --- apps/theming/lib/Migration/ThemingImages.php | 13 +- .../theming/tests/Migration/ThemingImages.php | 137 ++++++++++++++++++ 2 files changed, 143 insertions(+), 7 deletions(-) create mode 100644 apps/theming/tests/Migration/ThemingImages.php diff --git a/apps/theming/lib/Migration/ThemingImages.php b/apps/theming/lib/Migration/ThemingImages.php index 9627697f666..f6f484be8b5 100644 --- a/apps/theming/lib/Migration/ThemingImages.php +++ b/apps/theming/lib/Migration/ThemingImages.php @@ -37,7 +37,7 @@ class ThemingImages implements IRepairStep { private $appData; private $rootFolder; - public function __construct(IAppData $appData, ThemingDefaults $defaults, IRootFolder $rootFolder) { + public function __construct(IAppData $appData, IRootFolder $rootFolder) { $this->appData = $appData; $this->rootFolder = $rootFolder; } @@ -58,21 +58,20 @@ class ThemingImages implements IRepairStep { $file = null; try { $file = $this->rootFolder->get('themedinstancelogo'); - $logo = $folder->newFile("logo"); + $logo = $folder->newFile('logo'); $logo->putContent($file->getContent()); $file->delete(); } catch (NotFoundException $e) { - $output->info("No theming logo image to migrate"); + $output->info('No theming logo image to migrate'); } try { $file = $this->rootFolder->get('themedbackgroundlogo'); - $background = $folder->newFile("background"); + $background = $folder->newFile('background'); $background->putContent($file->getContent()); $file->delete(); } catch (NotFoundException $e) { - $output->info("No theming background image to migrate"); + $output->info('No theming background image to migrate'); } - } -} \ No newline at end of file +} diff --git a/apps/theming/tests/Migration/ThemingImages.php b/apps/theming/tests/Migration/ThemingImages.php new file mode 100644 index 00000000000..60cb6117755 --- /dev/null +++ b/apps/theming/tests/Migration/ThemingImages.php @@ -0,0 +1,137 @@ + + * + * @author Julius Härtl + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OCA\Theming\Tests\Migration; + +use OCP\Files\NotFoundException; +use OCP\Files\SimpleFS\ISimpleFolder; +use OCP\Migration\IOutput; +use Test\TestCase; +use OCA\Theming\Migration\ThemingImages; +use OCP\Files\IAppData; +use OCP\Files\IRootFolder; + +class ThemingImagesTest extends TestCase { + /** @var ThemingImages */ + private $repairStep; + /** @var IAppData */ + private $appData; + /** @var IRootFolder */ + private $rootFolder; + /** @var ISimpleFolder */ + private $imageFolder; + /** @var IOutput */ + private $output; + + public function setUp() { + parent::setUp(); + $this->appData = $this->getMockBuilder('OCP\Files\IAppData')->getMock(); + $this->rootFolder = $this->getMockBuilder('OCP\Files\IRootFolder')->getMock(); + $this->repairStep = new ThemingImages($this->appData, $this->rootFolder); + $this->imageFolder = $this->getMockBuilder('OCP\Files\SimpleFS\ISimpleFolder')->getMock(); + $this->output = $this->getMockBuilder('OCP\Migration\IOutput')->getMock(); + } + + public function testGetName() { + $this->assertEquals( + 'Move theming files to AppData storage', + $this->repairStep->getName() + ); + } + + public function testRunNoImages() { + $this->appData->expects($this->once()) + ->method('newFolder') + ->willReturn($this->imageFolder); + $this->rootFolder->expects($this->any()) + ->method('get') + ->willThrowException(new NotFoundException()); + $this->imageFolder->expects($this->never()) + ->method('newFile'); + $this->output->expects($this->exactly(2)) + ->method('info'); + $this->repairStep->run($this->output); + } + + public function testRunLogo() { + $oldFile = $this->getMockBuilder('OCP\Files\File')->getMock(); + $newFile = $this->getMockBuilder('OCP\Files\SimpleFS\ISimpleFile')->getMock(); + + $this->appData->expects($this->once()) + ->method('newFolder') + ->willReturn($this->imageFolder); + $this->rootFolder->expects($this->at(1)) + ->method('get') + ->with('themedbackgroundlogo') + ->willThrowException(new NotFoundException()); + $this->rootFolder->expects($this->at(0)) + ->method('get') + ->with('themedinstancelogo') + ->willReturn($oldFile); + $this->imageFolder->expects($this->once()) + ->method('newFile') + ->with('logo') + ->willReturn($newFile); + $oldFile->expects($this->once()) + ->method('getContent') + ->willReturn('data'); + $newFile->expects($this->once()) + ->method('putContent') + ->with('data'); + $oldFile->expects($this->once()) + ->method('delete'); + + $this->repairStep->run($this->output); + } + + public function testRunBackground() { + $oldFile = $this->getMockBuilder('OCP\Files\File')->getMock(); + $newFile = $this->getMockBuilder('OCP\Files\SimpleFS\ISimpleFile')->getMock(); + + $this->appData->expects($this->once()) + ->method('newFolder') + ->willReturn($this->imageFolder); + $this->rootFolder->expects($this->at(1)) + ->method('get') + ->with('themedbackgroundlogo') + ->willReturn($oldFile); + $this->rootFolder->expects($this->at(0)) + ->method('get') + ->with('themedinstancelogo') + ->willThrowException(new NotFoundException()); + $this->imageFolder->expects($this->once()) + ->method('newFile') + ->with('background') + ->willReturn($newFile); + $oldFile->expects($this->once()) + ->method('getContent') + ->willReturn('data'); + $newFile->expects($this->once()) + ->method('putContent') + ->with('data'); + $oldFile->expects($this->once()) + ->method('delete'); + + $this->repairStep->run($this->output); + } +} From b2cbe3530daa04451195865afc177bc6bde6c359 Mon Sep 17 00:00:00 2001 From: Julius Haertl Date: Thu, 9 Feb 2017 17:12:00 +0100 Subject: [PATCH 5/7] Use ::class in tests and correct import Signed-off-by: Julius Haertl --- .../lib/Controller/ThemingController.php | 4 +- .../Controller/ThemingControllerTest.php | 38 ++++++++++--------- .../theming/tests/Migration/ThemingImages.php | 20 +++++----- 3 files changed, 34 insertions(+), 28 deletions(-) diff --git a/apps/theming/lib/Controller/ThemingController.php b/apps/theming/lib/Controller/ThemingController.php index 19999d1362d..2aa79df2464 100644 --- a/apps/theming/lib/Controller/ThemingController.php +++ b/apps/theming/lib/Controller/ThemingController.php @@ -31,9 +31,9 @@ use OCA\Theming\ThemingDefaults; use OCP\AppFramework\Controller; use OCP\AppFramework\Http; use OCP\AppFramework\Http\DataDownloadResponse; +use OCP\AppFramework\Http\FileDisplayResponse; use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\Http\NotFoundResponse; -use OCP\AppFramework\Http\FileDisplayResponse; use OCP\AppFramework\Utility\ITimeFactory; use OCP\Files\File; use OCP\Files\IAppData; @@ -266,7 +266,7 @@ class ThemingController extends Controller { * @PublicPage * @NoCSRFRequired * - * @return Http\FileDisplayResponse|NotFoundResponse + * @return FileDisplayResponse|NotFoundResponse */ public function getLogo() { try { diff --git a/apps/theming/tests/Controller/ThemingControllerTest.php b/apps/theming/tests/Controller/ThemingControllerTest.php index 97770e7ce62..0c47ac972b7 100644 --- a/apps/theming/tests/Controller/ThemingControllerTest.php +++ b/apps/theming/tests/Controller/ThemingControllerTest.php @@ -24,14 +24,18 @@ */ namespace OCA\Theming\Tests\Controller; +use OC\L10N\L10N; use OCA\Theming\Controller\ThemingController; use OCA\Theming\Util; use OCP\App\IAppManager; use OCP\AppFramework\Http; use OCP\AppFramework\Http\DataResponse; +use OCP\AppFramework\Utility\ITimeFactory; use OCP\Files\IAppData; use OCP\Files\IRootFolder; use OCP\Files\NotFoundException; +use OCP\Files\SimpleFS\ISimpleFile; +use OCP\Files\SimpleFS\ISimpleFolder; use OCP\IConfig; use OCP\IL10N; use OCP\IRequest; @@ -64,22 +68,22 @@ class ThemingControllerTest extends TestCase { private $appData; public function setUp() { - $this->request = $this->getMockBuilder('OCP\IRequest')->getMock(); - $this->config = $this->getMockBuilder('OCP\IConfig')->getMock(); - $this->template = $this->getMockBuilder('OCA\Theming\ThemingDefaults') + $this->request = $this->getMockBuilder(IRequest::class)->getMock(); + $this->config = $this->getMockBuilder(IConfig::class)->getMock(); + $this->template = $this->getMockBuilder(ThemingDefaults::class) ->disableOriginalConstructor()->getMock(); - $this->timeFactory = $this->getMockBuilder('OCP\AppFramework\Utility\ITimeFactory') + $this->timeFactory = $this->getMockBuilder(ITimeFactory::class) ->disableOriginalConstructor() ->getMock(); - $this->l10n = $this->getMockBuilder('OCP\IL10N')->getMock(); - $this->rootFolder = $this->getMockBuilder('OCP\Files\IRootFolder')->getMock(); - $this->appManager = $this->getMockBuilder('OCP\App\IAppManager')->getMock(); + $this->l10n = $this->getMockBuilder(L10N::class)->getMock(); + $this->rootFolder = $this->getMockBuilder(IRootFolder::class)->getMock(); + $this->appManager = $this->getMockBuilder(IAppManager::class)->getMock(); $this->util = new Util($this->config, $this->rootFolder, $this->appManager); $this->timeFactory->expects($this->any()) ->method('getTime') ->willReturn(123); $this->tempManager = \OC::$server->getTempManager(); - $this->appData = $this->getMockBuilder('OCP\Files\IAppData')->getMock(); + $this->appData = $this->getMockBuilder(IAppData::class)->getMock(); $this->themingController = new ThemingController( 'theming', @@ -196,10 +200,10 @@ class ThemingControllerTest extends TestCase { ->willReturn('Saved'); - $file = $this->getMockBuilder('OCP\Files\SimpleFS\ISimpleFile') + $file = $this->getMockBuilder(ISimpleFile::class) ->disableOriginalConstructor() ->getMock(); - $folder = $this->getMockBuilder('OCP\Files\SimpleFS\ISimpleFolder') + $folder = $this->getMockBuilder(ISimpleFolder::class) ->disableOriginalConstructor() ->getMock(); $this->appData @@ -251,10 +255,10 @@ class ThemingControllerTest extends TestCase { ->with('Saved') ->willReturn('Saved'); - $file = $this->getMockBuilder('OCP\Files\SimpleFS\ISimpleFile') + $file = $this->getMockBuilder(ISimpleFile::class) ->disableOriginalConstructor() ->getMock(); - $folder = $this->getMockBuilder('OCP\Files\SimpleFS\ISimpleFolder') + $folder = $this->getMockBuilder(ISimpleFolder::class) ->disableOriginalConstructor() ->getMock(); $this->appData @@ -305,7 +309,7 @@ class ThemingControllerTest extends TestCase { ->with('Unsupported image type') ->willReturn('Unsupported image type'); - $folder = $this->getMockBuilder('OCP\Files\SimpleFS\ISimpleFolder') + $folder = $this->getMockBuilder(ISimpleFolder::class) ->disableOriginalConstructor() ->getMock(); $this->appData @@ -362,10 +366,10 @@ class ThemingControllerTest extends TestCase { } public function testGetLogo() { - $file = $this->getMockBuilder('OCP\Files\SimpleFS\ISimpleFile') + $file = $this->getMockBuilder(ISimpleFile::class) ->disableOriginalConstructor() ->getMock(); - $folder = $this->getMockBuilder('OCP\Files\SimpleFS\ISimpleFolder') + $folder = $this->getMockBuilder(ISimpleFolder::class) ->disableOriginalConstructor() ->getMock(); $this->appData @@ -405,10 +409,10 @@ class ThemingControllerTest extends TestCase { } public function testGetLoginBackground() { - $file = $this->getMockBuilder('OCP\Files\SimpleFS\ISimpleFile') + $file = $this->getMockBuilder(ISimpleFile::class) ->disableOriginalConstructor() ->getMock(); - $folder = $this->getMockBuilder('OCP\Files\SimpleFS\ISimpleFolder') + $folder = $this->getMockBuilder(ISimpleFolder::class) ->disableOriginalConstructor() ->getMock(); $this->appData diff --git a/apps/theming/tests/Migration/ThemingImages.php b/apps/theming/tests/Migration/ThemingImages.php index 60cb6117755..e5e3b1df5d3 100644 --- a/apps/theming/tests/Migration/ThemingImages.php +++ b/apps/theming/tests/Migration/ThemingImages.php @@ -23,7 +23,9 @@ namespace OCA\Theming\Tests\Migration; +use OCP\Files\File; use OCP\Files\NotFoundException; +use OCP\Files\SimpleFS\ISimpleFile; use OCP\Files\SimpleFS\ISimpleFolder; use OCP\Migration\IOutput; use Test\TestCase; @@ -45,11 +47,11 @@ class ThemingImagesTest extends TestCase { public function setUp() { parent::setUp(); - $this->appData = $this->getMockBuilder('OCP\Files\IAppData')->getMock(); - $this->rootFolder = $this->getMockBuilder('OCP\Files\IRootFolder')->getMock(); + $this->appData = $this->getMockBuilder(IAppData::class)->getMock(); + $this->rootFolder = $this->getMockBuilder(IRootFolder::class)->getMock(); $this->repairStep = new ThemingImages($this->appData, $this->rootFolder); - $this->imageFolder = $this->getMockBuilder('OCP\Files\SimpleFS\ISimpleFolder')->getMock(); - $this->output = $this->getMockBuilder('OCP\Migration\IOutput')->getMock(); + $this->imageFolder = $this->getMockBuilder(ISimpleFolder::class)->getMock(); + $this->output = $this->getMockBuilder(IOutput::class)->getMock(); } public function testGetName() { @@ -74,8 +76,8 @@ class ThemingImagesTest extends TestCase { } public function testRunLogo() { - $oldFile = $this->getMockBuilder('OCP\Files\File')->getMock(); - $newFile = $this->getMockBuilder('OCP\Files\SimpleFS\ISimpleFile')->getMock(); + $oldFile = $this->getMockBuilder(File::class)->getMock(); + $newFile = $this->getMockBuilder(ISimpleFile::class)->getMock(); $this->appData->expects($this->once()) ->method('newFolder') @@ -105,9 +107,9 @@ class ThemingImagesTest extends TestCase { } public function testRunBackground() { - $oldFile = $this->getMockBuilder('OCP\Files\File')->getMock(); - $newFile = $this->getMockBuilder('OCP\Files\SimpleFS\ISimpleFile')->getMock(); - + $oldFile = $this->getMockBuilder(File::class)->getMock(); + $newFile = $this->getMockBuilder(ISimpleFile::class)->getMock(); + $this->appData->expects($this->once()) ->method('newFolder') ->willReturn($this->imageFolder); From 3f0134622dafe01d1bed2c7d25d14da425e35fc4 Mon Sep 17 00:00:00 2001 From: Julius Haertl Date: Thu, 9 Feb 2017 22:51:05 +0100 Subject: [PATCH 6/7] Use createMock Signed-off-by: Julius Haertl --- .../Controller/ThemingControllerTest.php | 55 ++++++------------- .../theming/tests/Migration/ThemingImages.php | 16 +++--- 2 files changed, 25 insertions(+), 46 deletions(-) diff --git a/apps/theming/tests/Controller/ThemingControllerTest.php b/apps/theming/tests/Controller/ThemingControllerTest.php index 0c47ac972b7..38cd8c96202 100644 --- a/apps/theming/tests/Controller/ThemingControllerTest.php +++ b/apps/theming/tests/Controller/ThemingControllerTest.php @@ -68,22 +68,19 @@ class ThemingControllerTest extends TestCase { private $appData; public function setUp() { - $this->request = $this->getMockBuilder(IRequest::class)->getMock(); - $this->config = $this->getMockBuilder(IConfig::class)->getMock(); - $this->template = $this->getMockBuilder(ThemingDefaults::class) - ->disableOriginalConstructor()->getMock(); - $this->timeFactory = $this->getMockBuilder(ITimeFactory::class) - ->disableOriginalConstructor() - ->getMock(); - $this->l10n = $this->getMockBuilder(L10N::class)->getMock(); - $this->rootFolder = $this->getMockBuilder(IRootFolder::class)->getMock(); - $this->appManager = $this->getMockBuilder(IAppManager::class)->getMock(); + $this->request = $this->createMock(IRequest::class); + $this->config = $this->createMock(IConfig::class); + $this->template = $this->createMock(ThemingDefaults::class); + $this->timeFactory = $this->createMock(ITimeFactory::class); + $this->l10n = $this->createMock(L10N::class); + $this->rootFolder = $this->createMock(IRootFolder::class); + $this->appManager = $this->createMock(IAppManager::class); $this->util = new Util($this->config, $this->rootFolder, $this->appManager); $this->timeFactory->expects($this->any()) ->method('getTime') ->willReturn(123); $this->tempManager = \OC::$server->getTempManager(); - $this->appData = $this->getMockBuilder(IAppData::class)->getMock(); + $this->appData = $this->createMock(IAppData::class); $this->themingController = new ThemingController( 'theming', @@ -200,12 +197,8 @@ class ThemingControllerTest extends TestCase { ->willReturn('Saved'); - $file = $this->getMockBuilder(ISimpleFile::class) - ->disableOriginalConstructor() - ->getMock(); - $folder = $this->getMockBuilder(ISimpleFolder::class) - ->disableOriginalConstructor() - ->getMock(); + $file = $this->createMock(ISimpleFile::class); + $folder = $this->createMock(ISimpleFolder::class); $this->appData ->expects($this->once()) ->method('getFolder') @@ -255,12 +248,8 @@ class ThemingControllerTest extends TestCase { ->with('Saved') ->willReturn('Saved'); - $file = $this->getMockBuilder(ISimpleFile::class) - ->disableOriginalConstructor() - ->getMock(); - $folder = $this->getMockBuilder(ISimpleFolder::class) - ->disableOriginalConstructor() - ->getMock(); + $file = $this->createMock(ISimpleFile::class); + $folder = $this->createMock(ISimpleFolder::class); $this->appData ->expects($this->once()) ->method('getFolder') @@ -309,9 +298,7 @@ class ThemingControllerTest extends TestCase { ->with('Unsupported image type') ->willReturn('Unsupported image type'); - $folder = $this->getMockBuilder(ISimpleFolder::class) - ->disableOriginalConstructor() - ->getMock(); + $folder = $this->createMock(ISimpleFolder::class); $this->appData ->expects($this->once()) ->method('getFolder') @@ -366,12 +353,8 @@ class ThemingControllerTest extends TestCase { } public function testGetLogo() { - $file = $this->getMockBuilder(ISimpleFile::class) - ->disableOriginalConstructor() - ->getMock(); - $folder = $this->getMockBuilder(ISimpleFolder::class) - ->disableOriginalConstructor() - ->getMock(); + $file = $this->createMock(ISimpleFile::class); + $folder = $this->createMock(ISimpleFolder::class); $this->appData ->expects($this->once()) ->method('getFolder') @@ -409,12 +392,8 @@ class ThemingControllerTest extends TestCase { } public function testGetLoginBackground() { - $file = $this->getMockBuilder(ISimpleFile::class) - ->disableOriginalConstructor() - ->getMock(); - $folder = $this->getMockBuilder(ISimpleFolder::class) - ->disableOriginalConstructor() - ->getMock(); + $file = $this->createMock(ISimpleFile::class); + $folder = $this->createMock(ISimpleFolder::class); $this->appData ->expects($this->once()) ->method('getFolder') diff --git a/apps/theming/tests/Migration/ThemingImages.php b/apps/theming/tests/Migration/ThemingImages.php index e5e3b1df5d3..e15d0fd1bbc 100644 --- a/apps/theming/tests/Migration/ThemingImages.php +++ b/apps/theming/tests/Migration/ThemingImages.php @@ -47,11 +47,11 @@ class ThemingImagesTest extends TestCase { public function setUp() { parent::setUp(); - $this->appData = $this->getMockBuilder(IAppData::class)->getMock(); - $this->rootFolder = $this->getMockBuilder(IRootFolder::class)->getMock(); + $this->appData = $this->createMock(IAppData::class); + $this->rootFolder = $this->createMock(IRootFolder::class); $this->repairStep = new ThemingImages($this->appData, $this->rootFolder); - $this->imageFolder = $this->getMockBuilder(ISimpleFolder::class)->getMock(); - $this->output = $this->getMockBuilder(IOutput::class)->getMock(); + $this->imageFolder = $this->createMock(ISimpleFolder::class); + $this->output = $this->createMock(IOutput::class); } public function testGetName() { @@ -76,8 +76,8 @@ class ThemingImagesTest extends TestCase { } public function testRunLogo() { - $oldFile = $this->getMockBuilder(File::class)->getMock(); - $newFile = $this->getMockBuilder(ISimpleFile::class)->getMock(); + $oldFile = $this->createMock(File::class); + $newFile = $this->createMock(ISimpleFile::class); $this->appData->expects($this->once()) ->method('newFolder') @@ -107,8 +107,8 @@ class ThemingImagesTest extends TestCase { } public function testRunBackground() { - $oldFile = $this->getMockBuilder(File::class)->getMock(); - $newFile = $this->getMockBuilder(ISimpleFile::class)->getMock(); + $oldFile = $this->createMock(File::class); + $newFile = $this->createMock(ISimpleFile::class); $this->appData->expects($this->once()) ->method('newFolder') From e5ddb40a3ec32bc175f44ff613352a3569606746 Mon Sep 17 00:00:00 2001 From: Julius Haertl Date: Thu, 16 Feb 2017 15:13:19 +0100 Subject: [PATCH 7/7] Add test for creating AppData folder Signed-off-by: Julius Haertl --- .../Controller/ThemingControllerTest.php | 60 +++++++++++++++---- 1 file changed, 47 insertions(+), 13 deletions(-) diff --git a/apps/theming/tests/Controller/ThemingControllerTest.php b/apps/theming/tests/Controller/ThemingControllerTest.php index 38cd8c96202..3afcdb847b6 100644 --- a/apps/theming/tests/Controller/ThemingControllerTest.php +++ b/apps/theming/tests/Controller/ThemingControllerTest.php @@ -171,7 +171,15 @@ class ThemingControllerTest extends TestCase { $this->assertEquals($expected, $this->themingController->updateLogo()); } - public function testUpdateLogoNormalLogoUpload() { + public function dataUpdateImages() { + return [ + [false], + [true] + ]; + } + + /** @dataProvider dataUpdateImages */ + public function testUpdateLogoNormalLogoUpload($folderExists) { $tmpLogo = \OC::$server->getTempManager()->getTemporaryFolder() . '/logo.svg'; $destination = \OC::$server->getTempManager()->getTemporaryFolder(); @@ -199,16 +207,28 @@ class ThemingControllerTest extends TestCase { $file = $this->createMock(ISimpleFile::class); $folder = $this->createMock(ISimpleFolder::class); - $this->appData - ->expects($this->once()) - ->method('getFolder') - ->with('images') - ->willReturn($folder); + if($folderExists) { + $this->appData + ->expects($this->once()) + ->method('getFolder') + ->with('images') + ->willReturn($folder); + } else { + $this->appData + ->expects($this->at(0)) + ->method('getFolder') + ->with('images') + ->willThrowException(new NotFoundException()); + $this->appData + ->expects($this->at(1)) + ->method('newFolder') + ->with('images') + ->willReturn($folder); + } $folder->expects($this->once()) ->method('newFile') ->with('logo') ->willReturn($file); - $expected = new DataResponse( [ 'data' => @@ -223,7 +243,8 @@ class ThemingControllerTest extends TestCase { $this->assertEquals($expected, $this->themingController->updateLogo()); } - public function testUpdateLogoLoginScreenUpload() { + /** @dataProvider dataUpdateImages */ + public function testUpdateLogoLoginScreenUpload($folderExists) { $tmpLogo = \OC::$server->getTempManager()->getTemporaryFolder() . '/logo.svg'; touch($tmpLogo); @@ -250,11 +271,24 @@ class ThemingControllerTest extends TestCase { $file = $this->createMock(ISimpleFile::class); $folder = $this->createMock(ISimpleFolder::class); - $this->appData - ->expects($this->once()) - ->method('getFolder') - ->with('images') - ->willReturn($folder); + if($folderExists) { + $this->appData + ->expects($this->once()) + ->method('getFolder') + ->with('images') + ->willReturn($folder); + } else { + $this->appData + ->expects($this->at(0)) + ->method('getFolder') + ->with('images') + ->willThrowException(new NotFoundException()); + $this->appData + ->expects($this->at(1)) + ->method('newFolder') + ->with('images') + ->willReturn($folder); + } $folder->expects($this->once()) ->method('newFile') ->with('background')