diff --git a/apps/files/tests/Controller/ApiControllerTest.php b/apps/files/tests/Controller/ApiControllerTest.php index 4b7bec065a..56d0f6c8de 100644 --- a/apps/files/tests/Controller/ApiControllerTest.php +++ b/apps/files/tests/Controller/ApiControllerTest.php @@ -28,11 +28,15 @@ namespace OCA\Files\Controller; use OC\Files\FileInfo; use OCP\AppFramework\Http; +use OCP\Files\File; +use OCP\Files\Folder; use OCP\Files\NotFoundException; +use OCP\Files\SimpleFS\ISimpleFile; use OCP\Files\StorageNotAvailableException; use OCP\IConfig; use OCP\IUser; use OCP\IUserSession; +use OCP\Share\IManager; use Test\TestCase; use OCP\IRequest; use OCA\Files\Service\TagService; @@ -54,7 +58,7 @@ class ApiControllerTest extends TestCase { private $request; /** @var TagService */ private $tagService; - /** @var IPreview */ + /** @var IPreview|\PHPUnit_Framework_MockObject_MockObject */ private $preview; /** @var ApiController */ private $apiController; @@ -62,11 +66,13 @@ class ApiControllerTest extends TestCase { private $shareManager; /** @var \OCP\IConfig */ private $config; - /** @var \OC\Files\Node\Folder */ + /** @var Folder|\PHPUnit_Framework_MockObject_MockObject */ private $userFolder; public function setUp() { - $this->request = $this->getMockBuilder('\OCP\IRequest') + parent::setUp(); + + $this->request = $this->getMockBuilder(IRequest::class) ->disableOriginalConstructor() ->getMock(); $this->user = $this->createMock(IUser::class); @@ -77,17 +83,17 @@ class ApiControllerTest extends TestCase { $userSession->expects($this->any()) ->method('getUser') ->will($this->returnValue($this->user)); - $this->tagService = $this->getMockBuilder('\OCA\Files\Service\TagService') + $this->tagService = $this->getMockBuilder(TagService::class) ->disableOriginalConstructor() ->getMock(); - $this->shareManager = $this->getMockBuilder('\OCP\Share\IManager') + $this->shareManager = $this->getMockBuilder(IManager::class) ->disableOriginalConstructor() ->getMock(); - $this->preview = $this->getMockBuilder('\OCP\IPreview') + $this->preview = $this->getMockBuilder(IPreview::class) ->disableOriginalConstructor() ->getMock(); $this->config = $this->createMock(IConfig::class); - $this->userFolder = $this->getMockBuilder('\OC\Files\Node\Folder') + $this->userFolder = $this->getMockBuilder(Folder::class) ->disableOriginalConstructor() ->getMock(); @@ -153,28 +159,41 @@ class ApiControllerTest extends TestCase { } public function testGetThumbnailInvalidSize() { + $this->userFolder->method('get') + ->with($this->equalTo('')) + ->willThrowException(new NotFoundException()); $expected = new DataResponse(['message' => 'Requested size must be numeric and a positive value.'], Http::STATUS_BAD_REQUEST); $this->assertEquals($expected, $this->apiController->getThumbnail(0, 0, '')); } public function testGetThumbnailInvaidImage() { + $file = $this->createMock(File::class); + $this->userFolder->method('get') + ->with($this->equalTo('unknown.jpg')) + ->willReturn($file); $this->preview->expects($this->once()) - ->method('createPreview') - ->with('files/unknown.jpg', 10, 10, true) - ->willReturn(new Image); + ->method('getPreview') + ->with($file, 10, 10, true) + ->willThrowException(new NotFoundException()); $expected = new DataResponse(['message' => 'File not found.'], Http::STATUS_NOT_FOUND); $this->assertEquals($expected, $this->apiController->getThumbnail(10, 10, 'unknown.jpg')); } public function testGetThumbnail() { + $file = $this->createMock(File::class); + $this->userFolder->method('get') + ->with($this->equalTo('known.jpg')) + ->willReturn($file); + $preview = $this->createMock(ISimpleFile::class); $this->preview->expects($this->once()) - ->method('createPreview') - ->with('files/known.jpg', 10, 10, true) - ->willReturn(new Image(\OC::$SERVERROOT.'/tests/data/testimage.jpg')); + ->method('getPreview') + ->with($this->equalTo($file), 10, 10, true) + ->willReturn($preview); $ret = $this->apiController->getThumbnail(10, 10, 'known.jpg'); $this->assertEquals(Http::STATUS_OK, $ret->getStatus()); + $this->assertInstanceOf(Http\FileDisplayResponse::class, $ret); } public function testUpdateFileSorting() { diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 19905bdbe7..42cfb8c45e 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -622,6 +622,7 @@ return array( 'OC\\Preview\\Font' => $baseDir . '/lib/private/Preview/Font.php', 'OC\\Preview\\GIF' => $baseDir . '/lib/private/Preview/GIF.php', 'OC\\Preview\\Generator' => $baseDir . '/lib/private/Preview/Generator.php', + 'OC\\Preview\\GeneratorHelper' => $baseDir . '/lib/private/Preview/GeneratorHelper.php', 'OC\\Preview\\Illustrator' => $baseDir . '/lib/private/Preview/Illustrator.php', 'OC\\Preview\\Image' => $baseDir . '/lib/private/Preview/Image.php', 'OC\\Preview\\JPEG' => $baseDir . '/lib/private/Preview/JPEG.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 82dba9c92f..d7e937577f 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -652,6 +652,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c 'OC\\Preview\\Font' => __DIR__ . '/../../..' . '/lib/private/Preview/Font.php', 'OC\\Preview\\GIF' => __DIR__ . '/../../..' . '/lib/private/Preview/GIF.php', 'OC\\Preview\\Generator' => __DIR__ . '/../../..' . '/lib/private/Preview/Generator.php', + 'OC\\Preview\\GeneratorHelper' => __DIR__ . '/../../..' . '/lib/private/Preview/GeneratorHelper.php', 'OC\\Preview\\Illustrator' => __DIR__ . '/../../..' . '/lib/private/Preview/Illustrator.php', 'OC\\Preview\\Image' => __DIR__ . '/../../..' . '/lib/private/Preview/Image.php', 'OC\\Preview\\JPEG' => __DIR__ . '/../../..' . '/lib/private/Preview/JPEG.php', diff --git a/lib/private/Preview/Generator.php b/lib/private/Preview/Generator.php index d4c38b1cb6..3d4e9bf367 100644 --- a/lib/private/Preview/Generator.php +++ b/lib/private/Preview/Generator.php @@ -23,46 +23,43 @@ namespace OC\Preview; -use OC\Files\View; use OCP\Files\File; 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\IImage; -use OCP\Image as img; use OCP\IPreview; use OCP\Preview\IProvider; class Generator { - /** @var IRootFolder*/ - private $rootFolder; /** @var IPreview */ private $previewManager; /** @var IConfig */ private $config; /** @var IAppData */ private $appData; + /** @var GeneratorHelper */ + private $helper; /** - * @param IRootFolder $rootFolder * @param IConfig $config * @param IPreview $previewManager * @param IAppData $appData + * @param GeneratorHelper $helper */ public function __construct( - IRootFolder $rootFolder, IConfig $config, IPreview $previewManager, - IAppData $appData + IAppData $appData, + GeneratorHelper $helper ) { - $this->rootFolder = $rootFolder; $this->config = $config; $this->previewManager = $previewManager; $this->appData = $appData; + $this->helper = $helper; } /** @@ -88,10 +85,6 @@ class Generator { throw new NotFoundException(); } - /* - * Get the preview folder - * TODO: Separate preview creation from storing previews - */ $previewFolder = $this->getPreviewFolder($file); // Get the max preview and infer the max preview sizes from that @@ -134,17 +127,15 @@ class Generator { } foreach ($providers as $provider) { - $provider = $provider(); + $provider = $this->helper->getProvider($provider); if (!($provider instanceof IProvider)) { continue; } - list($view, $path) = $this->getViewAndPath($file); - $maxWidth = (int)$this->config->getSystemValue('preview_max_x', 2048); $maxHeight = (int)$this->config->getSystemValue('preview_max_y', 2048); - $preview = $provider->getThumbnail($path, $maxWidth, $maxHeight, false, $view); + $preview = $this->helper->getThumbnail($provider, $file, $maxWidth, $maxHeight); if (!($preview instanceof IImage)) { continue; @@ -185,24 +176,7 @@ class Generator { return $path; } - /** - * @param File $file - * @return array - * This is required to create the old view and path - */ - private function getViewAndPath(File $file) { - $owner = $file->getOwner()->getUID(); - $userFolder = $this->rootFolder->getUserFolder($owner)->getParent(); - $nodes = $userFolder->getById($file->getId()); - - $file = $nodes[0]; - - $view = new View($userFolder->getPath()); - $path = $userFolder->getRelativePath($file->getPath()); - - return [$view, $path]; - } /** * @param int $width @@ -300,7 +274,7 @@ class Generator { * @throws NotFoundException */ private function generatePreview(ISimpleFolder $previewFolder, ISimpleFile $maxPreview, $width, $height, $crop, $maxWidth, $maxHeight) { - $preview = new img($maxPreview->getContent()); + $preview = $this->helper->getImage($maxPreview); if ($crop) { if ($height !== $preview->height() && $width !== $preview->width()) { diff --git a/lib/private/Preview/GeneratorHelper.php b/lib/private/Preview/GeneratorHelper.php new file mode 100644 index 0000000000..282c46a2a5 --- /dev/null +++ b/lib/private/Preview/GeneratorHelper.php @@ -0,0 +1,91 @@ + + * + * @author Roeland Jago Douma + * + * @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 OC\Preview; + +use OC\Files\View; +use OCP\Files\File; +use OCP\Files\IRootFolder; +use OCP\Files\SimpleFS\ISimpleFile; +use OCP\IImage; +use OCP\Image as img; +use OCP\Preview\IProvider; + +/** + * Very small wrapper class to make the generator fully unit testable + */ +class GeneratorHelper { + + /** @var IRootFolder */ + private $rootFolder; + + public function __construct(IRootFolder $rootFolder) { + $this->rootFolder = $rootFolder; + } + + /** + * @param IProvider $provider + * @param File $file + * @param int $maxWidth + * @param int $maxHeight + * @return bool|IImage + */ + public function getThumbnail(IProvider $provider, File $file, $maxWidth, $maxHeight) { + list($view, $path) = $this->getViewAndPath($file); + return $provider->getThumbnail($path, $maxWidth, $maxHeight, false, $view); + } + + /** + * @param File $file + * @return array + * This is required to create the old view and path + */ + private function getViewAndPath(File $file) { + $owner = $file->getOwner()->getUID(); + + $userFolder = $this->rootFolder->getUserFolder($owner)->getParent(); + + $nodes = $userFolder->getById($file->getId()); + $file = $nodes[0]; + + $view = new View($userFolder->getPath()); + $path = $userFolder->getRelativePath($file->getPath()); + + return [$view, $path]; + } + + /** + * @param ISimpleFile $maxPreview + * @return IImage + */ + public function getImage(ISimpleFile $maxPreview) { + return new img($maxPreview->getContent()); + } + + /** + * @param $provider + * @return IProvider + */ + public function getProvider($provider) { + return $provider(); + } +} diff --git a/lib/private/PreviewManager.php b/lib/private/PreviewManager.php index 2c17a5f3c8..a2ef9659b3 100644 --- a/lib/private/PreviewManager.php +++ b/lib/private/PreviewManager.php @@ -26,6 +26,7 @@ namespace OC; use OC\Preview\Generator; +use OC\Preview\GeneratorHelper; use OCP\Files\File; use OCP\Files\IAppData; use OCP\Files\IRootFolder; @@ -159,10 +160,12 @@ class PreviewManager implements IPreview { public function getPreview(File $file, $width = -1, $height = -1, $crop = false, $mode = IPreview::MODE_FILL, $mimeType = null) { if ($this->generator === null) { $this->generator = new Generator( - $this->rootFolder, $this->config, $this, - $this->appData + $this->appData, + new GeneratorHelper( + $this->rootFolder + ) ); } diff --git a/tests/lib/Preview/GeneratorTest.php b/tests/lib/Preview/GeneratorTest.php new file mode 100644 index 0000000000..d64a0b912e --- /dev/null +++ b/tests/lib/Preview/GeneratorTest.php @@ -0,0 +1,338 @@ + + * + * @author Roeland Jago Douma + * + * @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 Test\Preview; + +use OC\Preview\Generator; +use OC\Preview\GeneratorHelper; +use OCP\Files\File; +use OCP\Files\IAppData; +use OCP\Files\NotFoundException; +use OCP\Files\SimpleFS\ISimpleFile; +use OCP\Files\SimpleFS\ISimpleFolder; +use OCP\IConfig; +use OCP\IImage; +use OCP\IPreview; +use OCP\Preview\IProvider; + +class GeneratorTest extends \Test\TestCase { + + /** @var IConfig|\PHPUnit_Framework_MockObject_MockObject */ + private $config; + + /** @var IPreview|\PHPUnit_Framework_MockObject_MockObject */ + private $previewManager; + + /** @var IAppData|\PHPUnit_Framework_MockObject_MockObject */ + private $appData; + + /** @var GeneratorHelper|\PHPUnit_Framework_MockObject_MockObject */ + private $helper; + + /** @var Generator */ + private $generator; + + public function setUp() { + parent::setUp(); + + $this->config = $this->createMock(IConfig::class); + $this->previewManager = $this->createMock(IPreview::class); + $this->appData = $this->createMock(IAppData::class); + $this->helper = $this->createMock(GeneratorHelper::class); + + $this->generator = new Generator( + $this->config, + $this->previewManager, + $this->appData, + $this->helper + ); + } + + public function testGetCachedPreview() { + $file = $this->createMock(File::class); + $file->method('getMimeType') + ->willReturn('myMimeType'); + $file->method('getId') + ->willReturn(42); + + $this->previewManager->method('isMimeSupported') + ->with($this->equalTo('myMimeType')) + ->willReturn(true); + + $previewFolder = $this->createMock(ISimpleFolder::class); + $this->appData->method('getFolder') + ->with($this->equalTo(42)) + ->willReturn($previewFolder); + + $maxPreview = $this->createMock(ISimpleFile::class); + $maxPreview->method('getName') + ->willReturn('1000-1000-max.png'); + + $previewFolder->method('getDirectoryListing') + ->willReturn([$maxPreview]); + + $previewFile = $this->createMock(ISimpleFile::class); + + $previewFolder->method('getFile') + ->with($this->equalTo('128-128.png')) + ->willReturn($previewFile); + + $result = $this->generator->getPreview($file, 100, 100); + $this->assertSame($previewFile, $result); + } + + public function testGetNewPreview() { + $file = $this->createMock(File::class); + $file->method('getMimeType') + ->willReturn('myMimeType'); + $file->method('getId') + ->willReturn(42); + + $this->previewManager->method('isMimeSupported') + ->with($this->equalTo('myMimeType')) + ->willReturn(true); + + $previewFolder = $this->createMock(ISimpleFolder::class); + $this->appData->method('getFolder') + ->with($this->equalTo(42)) + ->willThrowException(new NotFoundException()); + + $this->appData->method('newFolder') + ->with($this->equalTo(42)) + ->willReturn($previewFolder); + + $this->config->method('getSystemValue') + ->will($this->returnCallback(function($key, $defult) { + return $defult; + })); + + $invalidProvider = $this->createMock(IProvider::class); + $validProvider = $this->createMock(IProvider::class); + + $this->previewManager->method('getProviders') + ->willReturn([ + '/image\/png/' => ['wrongProvider'], + '/myMimeType/' => ['brokenProvider', 'invalidProvider', 'validProvider'], + ]); + + $this->helper->method('getProvider') + ->will($this->returnCallback(function($provider) use ($invalidProvider, $validProvider) { + if ($provider === 'wrongProvider') { + $this->fail('Wrongprovider should not be constructed!'); + } else if ($provider === 'brokenProvider') { + return false; + } else if ($provider === 'invalidProvider') { + return $invalidProvider; + } else if ($provider === 'validProvider') { + return $validProvider; + } + $this->fail('Unexpected provider requested'); + })); + + $image = $this->createMock(IImage::class); + $image->method('width')->willReturn(2048); + $image->method('height')->willReturn(2048); + + $this->helper->method('getThumbnail') + ->will($this->returnCallback(function ($provider, $file, $x, $y) use ($invalidProvider, $validProvider, $image) { + if ($provider === $validProvider) { + return $image; + } else { + return false; + } + })); + + $image->method('data') + ->willReturn('my data'); + + $maxPreview = $this->createMock(ISimpleFile::class); + $maxPreview->method('getName')->willReturn('2048-2048-max.png'); + + $previewFile = $this->createMock(ISimpleFile::class); + + $previewFolder->method('getDirectoryListing') + ->willReturn([]); + $previewFolder->method('newFile') + ->will($this->returnCallback(function($filename) use ($maxPreview, $previewFile) { + if ($filename === '2048-2048-max.png') { + return $maxPreview; + } else if ($filename === '128-128.png') { + return $previewFile; + } + $this->fail('Unexpected file'); + })); + + $maxPreview->expects($this->once()) + ->method('putContent') + ->with($this->equalTo('my data')); + + $previewFolder->method('getFile') + ->with($this->equalTo('128-128.png')) + ->willThrowException(new NotFoundException()); + + $image = $this->createMock(IImage::class); + $this->helper->method('getImage') + ->with($this->equalTo($maxPreview)) + ->willReturn($image); + + $image->expects($this->once()) + ->method('resize') + ->with(128); + $image->method('data') + ->willReturn('my resized data'); + + $previewFile->expects($this->once()) + ->method('putContent') + ->with('my resized data'); + + $result = $this->generator->getPreview($file, 100, 100); + $this->assertSame($previewFile, $result); + } + + public function testInvalidMimeType() { + $this->expectException(NotFoundException::class); + + $file = $this->createMock(File::class); + + $this->previewManager->method('isMimeSupported') + ->with('invalidType') + ->willReturn(false); + + $this->generator->getPreview($file, 0, 0, true, IPreview::MODE_COVER, 'invalidType'); + } + + public function testNoProvider() { + $file = $this->createMock(File::class); + $file->method('getMimeType') + ->willReturn('myMimeType'); + $file->method('getId') + ->willReturn(42); + + $this->previewManager->method('isMimeSupported') + ->with($this->equalTo('myMimeType')) + ->willReturn(true); + + $previewFolder = $this->createMock(ISimpleFolder::class); + $this->appData->method('getFolder') + ->with($this->equalTo(42)) + ->willReturn($previewFolder); + + $previewFolder->method('getDirectoryListing') + ->willReturn([]); + + $this->previewManager->method('getProviders') + ->willReturn([]); + + $this->expectException(NotFoundException::class); + $this->generator->getPreview($file, 100, 100); + } + + public function dataSize() { + return [ + [1024, 2048, 512, 512, false, IPreview::MODE_FILL, 256, 512], + [1024, 2048, 512, 512, false, IPreview::MODE_COVER, 512, 1024], + [1024, 2048, 512, 512, true, IPreview::MODE_FILL, 512, 512], + [1024, 2048, 512, 512, true, IPreview::MODE_COVER, 512, 512], + + [1024, 2048, -1, 512, false, IPreview::MODE_COVER, 256, 512], + [1024, 2048, 512, -1, false, IPreview::MODE_FILL, 512, 1024], + + [1024, 2048, 250, 1100, true, IPreview::MODE_COVER, 256, 1126], + [1024, 1100, 250, 1100, true, IPreview::MODE_COVER, 250, 1100], + + [1024, 2048, 4096, 2048, false, IPreview::MODE_FILL, 1024, 2048], + [1024, 2048, 4096, 2048, false, IPreview::MODE_COVER, 1024, 2048], + + + [2048, 1024, 512, 512, false, IPreview::MODE_FILL, 512, 256], + [2048, 1024, 512, 512, false, IPreview::MODE_COVER, 1024, 512], + [2048, 1024, 512, 512, true, IPreview::MODE_FILL, 512, 512], + [2048, 1024, 512, 512, true, IPreview::MODE_COVER, 512, 512], + + [2048, 1024, -1, 512, false, IPreview::MODE_FILL, 1024, 512], + [2048, 1024, 512, -1, false, IPreview::MODE_COVER, 512, 256], + + [2048, 1024, 4096, 1024, true, IPreview::MODE_FILL, 2048, 512], + [2048, 1024, 4096, 1024, true, IPreview::MODE_COVER, 2048, 512], + ]; + } + + /** + * @dataProvider dataSize + * + * @param int $maxX + * @param int $maxY + * @param int $reqX + * @param int $reqY + * @param bool $crop + * @param string $mode + * @param int $expectedX + * @param int $expectedY + */ + public function testCorrectSize($maxX, $maxY, $reqX, $reqY, $crop, $mode, $expectedX, $expectedY) { + $file = $this->createMock(File::class); + $file->method('getMimeType') + ->willReturn('myMimeType'); + $file->method('getId') + ->willReturn(42); + + $this->previewManager->method('isMimeSupported') + ->with($this->equalTo('myMimeType')) + ->willReturn(true); + + $previewFolder = $this->createMock(ISimpleFolder::class); + $this->appData->method('getFolder') + ->with($this->equalTo(42)) + ->willReturn($previewFolder); + + $maxPreview = $this->createMock(ISimpleFile::class); + $maxPreview->method('getName') + ->willReturn($maxX . '-' . $maxY . '-max.png'); + + $previewFolder->method('getDirectoryListing') + ->willReturn([$maxPreview]); + + $filename = $expectedX . '-' . $expectedY; + if ($crop) { + $filename .= '-crop'; + } + $filename .= '.png'; + $previewFolder->method('getFile') + ->with($this->equalTo($filename)) + ->willThrowException(new NotFoundException()); + + $image = $this->createMock(IImage::class); + $this->helper->method('getImage') + ->with($this->equalTo($maxPreview)) + ->willReturn($image); + $image->method('height')->willReturn($maxY); + $image->method('width')->willReturn($maxX); + + $preview = $this->createMock(ISimpleFile::class); + $previewFolder->method('newFile') + ->with($this->equalTo($filename)) + ->willReturn($preview); + + $result = $this->generator->getPreview($file, $reqX, $reqY, $crop, $mode); + $this->assertSame($preview, $result); + } +}