From 7694d8088448b17499a4380a51d241adfae452ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= Date: Fri, 20 Oct 2017 20:25:07 +0200 Subject: [PATCH] Fix preview provider tests --- lib/private/Files/Node/Root.php | 2 +- lib/private/Preview.php | 116 +++++++++++++++----------------- lib/private/Preview/Office.php | 6 +- tests/lib/Preview/Provider.php | 15 +++-- tests/lib/PreviewTest.php | 61 +++++++++-------- 5 files changed, 101 insertions(+), 99 deletions(-) diff --git a/lib/private/Files/Node/Root.php b/lib/private/Files/Node/Root.php index e827c06fa58a..8e18abccc3b2 100644 --- a/lib/private/Files/Node/Root.php +++ b/lib/private/Files/Node/Root.php @@ -175,7 +175,7 @@ public function unMount($mount) { * @param string $path * @throws \OCP\Files\NotFoundException * @throws \OCP\Files\NotPermittedException - * @return string + * @return File|Folder */ public function get($path) { $path = $this->normalizePath($path); diff --git a/lib/private/Preview.php b/lib/private/Preview.php index 6a30620f9fae..1da7323c114c 100644 --- a/lib/private/Preview.php +++ b/lib/private/Preview.php @@ -34,8 +34,12 @@ use OC\Files\View; use OCP\Files\File; use OCP\Files\FileInfo; +use OCP\Files\Folder; +use OCP\Files\Node; use OCP\Files\NotFoundException; +use OCP\IImage; use OCP\Preview\IProvider2; +use OCP\Response; use OCP\Util; class Preview { @@ -84,7 +88,7 @@ class Preview { /** * preview images object * - * @var \OCP\IImage + * @var IImage */ private $preview; /** @var string */ @@ -95,22 +99,19 @@ class Preview { * * @param string $user userid - if no user is given, OC_User::getUser will be used * @param string $root path of root - * @param File $file The path to the file where you want a thumbnail from + * @param Node $file The path to the file where you want a thumbnail from * @param int $maxX The maximum X size of the thumbnail. It can be smaller depending on the * shape of the image * @param int $maxY The maximum Y size of the thumbnail. It can be smaller depending on the * shape of the image - * @param bool $scalingUp Disable/Enable upscaling of previews - * + * @param bool $scalingUp Disable/Enable up-scaling of previews + * @param string $versionId * @throws \Exception - * @return mixed (bool / string) - * false if thumbnail does not exist - * path to thumbnail if thumbnail exists */ public function __construct( $user = '', $root = '/', - $file = null, $maxX = 1, + Node $file = null, $maxX = 1, $maxY = 1, $scalingUp = true, $versionId = null @@ -234,7 +235,7 @@ protected function getFileInfo() { * @return array|null */ private function getChildren() { - $absPath = $this->fileView->getAbsolutePath($this->file); + $absPath = $this->file->getPath(); $absPath = Files\Filesystem::normalizePath($absPath); if (array_key_exists($absPath, self::$deleteChildrenMapper)) { @@ -247,13 +248,12 @@ private function getChildren() { /** * Sets the path of the file you want a preview of * - * @param File $file - * @param \OCP\Files\FileInfo|null $info + * @param Node $file * @param string $versionId * * @return Preview */ - public function setFile(File $file, $versionId = null) { + public function setFile(Node $file, $versionId = null) { $this->file = $file; $this->versionId = $versionId; $this->mimeType = $this->file->getMimetype(); @@ -413,7 +413,7 @@ public function deleteAllPreviews() { // getId() might return null, e.g. when the file is a // .ocTransferId*.part file from chunked file upload. if (!empty($fileId)) { - $previewPath = $this->getPreviewPath(); + $previewPath = $this->getPreviewPath($fileId); $this->userView->rmdir($previewPath); } } @@ -739,7 +739,7 @@ private function unscalable($x, $y) { * The cache is searched first and if nothing usable was found then a preview is * generated by one of the providers * - * @return \OCP\IImage + * @return IImage */ public function getPreview() { if (!is_null($this->preview) && $this->preview->valid()) { @@ -785,9 +785,9 @@ public function showPreview($mimeTypeForHeaders = null) { if (is_null($this->preview)) { $this->getPreview(); } - if ($this->preview instanceof \OCP\IImage) { + if ($this->preview instanceof IImage) { if ($this->preview->valid()) { - \OCP\Response::enableCaching(3600 * 24); // 24 hours + Response::enableCaching(3600 * 24); // 24 hours } else { $this->getMimeIcon(); } @@ -832,7 +832,7 @@ private function getCachedPreview($cached) { */ private function resizeAndStore() { $image = $this->preview; - if (!($image instanceof \OCP\IImage)) { + if (!($image instanceof IImage)) { Util::writeLog( 'core', '$this->preview is not an instance of \OCP\IImage', Util::DEBUG ); @@ -904,7 +904,7 @@ private function resizeAndStore() { * * The new dimensions can be larger or smaller than the ones of the preview we have to resize * - * @param \OCP\IImage $image + * @param IImage $image * @param int $askedWidth * @param int $askedHeight * @param int $previewWidth @@ -954,7 +954,7 @@ private function scale($image, $askedWidth, $askedHeight, $previewWidth, $previe /** * Crops a preview which is larger than the dimensions we've received * - * @param \OCP\IImage $image + * @param IImage $image * @param int $askedWidth * @param int $askedHeight * @param int $previewWidth @@ -973,7 +973,7 @@ private function crop($image, $askedWidth, $askedHeight, $previewWidth, $preview * Crops an image if it's larger than the dimensions we've received and fills the empty space * with a transparent background * - * @param \OCP\IImage $image + * @param IImage $image * @param int $askedWidth * @param int $askedHeight * @param int $previewWidth @@ -1075,11 +1075,13 @@ private function buildCachePath($maxX = null, $maxY = null) { * * @return string */ - private function getPreviewPath() { - $fileId = $this->getFileInfo()->getId(); - if ($this->versionId !== null) { - $fileId .= '/'; - $fileId .= $this->versionId; + private function getPreviewPath($fileId = null) { + if ($fileId === null) { + $fileId = $this->getFileInfo()->getId(); + if ($this->versionId !== null) { + $fileId .= '/'; + $fileId .= $this->versionId; + } } return $this->getThumbnailsFolder() . '/' . $fileId . '/'; } @@ -1119,7 +1121,7 @@ private function generatePreview() { $preview = $provider->getThumbnail($file, $this->configMaxWidth, $this->configMaxHeight, false); - if (!($preview instanceof \OCP\IImage)) { + if (!($preview instanceof IImage)) { continue; } @@ -1220,14 +1222,14 @@ private function limitMaxDim($dim, $maxDim, $dimName) { * @param array $args */ public static function post_write($args) { -// self::post_delete($args, 'files/'); + self::post_delete($args, 'files/'); } /** * @param array $args */ public static function prepare_delete_files($args) { -// self::prepare_delete($args, 'files/'); + self::prepare_delete($args, 'files/'); } /** @@ -1239,18 +1241,11 @@ public static function prepare_delete(array $args, $prefix = '') { if (substr($path, 0, 1) === '/') { $path = substr($path, 1); } - - $view = new View('/' . \OC_User::getUser() . '/' . $prefix); - - $absPath = Files\Filesystem::normalizePath($view->getAbsolutePath($path)); - $fileInfo = $view->getFileInfo($path); - if ($fileInfo === false) { - return; - } - self::addPathToDeleteFileMapper($absPath, $fileInfo); - if ($view->is_dir($path)) { - $children = self::getAllChildren($view, $path); - self::$deleteChildrenMapper[$absPath] = $children; + $node = \OC::$server->getUserFolder()->get($path); + self::addPathToDeleteFileMapper($node->getPath(), $node); + if ($node->getType() === FileInfo::TYPE_FOLDER) { + $children = self::getAllChildren($node); + self::$deleteChildrenMapper[$node->getPath()] = $children; } } @@ -1263,47 +1258,41 @@ private static function addPathToDeleteFileMapper($absolutePath, $info) { } /** - * @param View $view - * @param string $path + * @param Folder $node * * @return array */ - private static function getAllChildren($view, $path) { - $children = $view->getDirectoryContent($path); - $childrensFiles = []; - - $fakeRootLength = strlen($view->getRoot()); + private static function getAllChildren($node) { + $children = $node->getDirectoryListing(); + $childrenFiles = []; - for ($i = 0; $i < count($children); $i++) { - $child = $children[$i]; + foreach ($children as $child) { - $childsPath = substr($child->getPath(), $fakeRootLength); - - if ($view->is_dir($childsPath)) { - $children = array_merge( - $children, - $view->getDirectoryContent($childsPath) + if ($child->getType() === FileInfo::TYPE_FOLDER) { + $childrenFiles = array_merge( + $childrenFiles, + self::getAllChildren($child) ); } else { - $childrensFiles[] = $child; + $childrenFiles[] = $child; } } - return $childrensFiles; + return $childrenFiles; } /** * @param array $args */ public static function post_delete_files($args) { -// self::post_delete($args, 'files/'); + self::post_delete($args, 'files/'); } /** * @param array $args */ public static function post_delete_versions($args) { -// self::post_delete($args, 'files/'); + self::post_delete($args, 'files/'); } /** @@ -1314,7 +1303,14 @@ public static function post_delete($args, $prefix = '') { $path = Files\Filesystem::normalizePath($args['path']); $user = isset($args['user']) ? $args['user'] : \OC_User::getUser(); - $preview = new Preview($user, $prefix, $path); + $view = new View('/' . $user . '/' . $prefix); + $absPath = Files\Filesystem::normalizePath($view->getAbsolutePath($path)); + if (!isset(self::$deleteFileMapper[$absPath])) { + return; + } + $node = self::$deleteFileMapper[$absPath]; + + $preview = new Preview($user, $prefix, $node); $preview->deleteAllPreviews(); } diff --git a/lib/private/Preview/Office.php b/lib/private/Preview/Office.php index 4cdc31d814c5..6534094f419b 100644 --- a/lib/private/Preview/Office.php +++ b/lib/private/Preview/Office.php @@ -62,14 +62,14 @@ public function getThumbnail(File $file, $maxX, $maxY, $scalingUp) { //create imagick object from pdf $pdfPreview = null; try { - list($dirname, , , $filename) = array_values(pathinfo($absPath)); - $pdfPreview = $dirname . '/' . $filename . '.pdf'; + $pathInfo = pathinfo($absPath); + $pdfPreview = $pathInfo['dirname'] . '/' . $pathInfo['filename'] . '.pdf'; $pdf = new \imagick($pdfPreview . '[0]'); $pdf->setImageFormat('jpg'); } catch (\Exception $e) { unlink($absPath); - unlink($pdfPreview); + @unlink($pdfPreview); \OCP\Util::writeLog('core', $e->getmessage(), \OCP\Util::ERROR); return false; } diff --git a/tests/lib/Preview/Provider.php b/tests/lib/Preview/Provider.php index eebc24428c94..38225f40fc70 100644 --- a/tests/lib/Preview/Provider.php +++ b/tests/lib/Preview/Provider.php @@ -21,18 +21,20 @@ namespace Test\Preview; +use OCP\Files\Node; +use OCP\Preview\IProvider2; use Test\Traits\UserTrait; abstract class Provider extends \Test\TestCase { use UserTrait; - /** @var string */ + /** @var Node */ protected $imgPath; /** @var int */ protected $width; /** @var int */ protected $height; - /** @var \OC\Preview\Provider */ + /** @var IProvider2 */ protected $provider; /** @var int */ protected $maxWidth = 1024; @@ -116,7 +118,7 @@ public function testGetThumbnail($widthAdjustment, $heightAdjustment) { * @param string $fileName name of the file to create * @param string $fileContent path to file to use for test * - * @return string + * @return Node */ protected function prepareTestFile($fileName, $fileContent) { $imgData = file_get_contents($fileContent); @@ -125,19 +127,18 @@ protected function prepareTestFile($fileName, $fileContent) { $scanner = $this->storage->getScanner(); $scanner->scan(''); - - return $imgPath; + return \OC::$server->getUserFolder($this->userId)->get($fileName); } /** * Retrieves a max size thumbnail can be created * - * @param \OC\Preview\Provider $provider + * @param IProvider2 $provider * * @return bool|\OCP\IImage */ private function getPreview($provider) { - $preview = $provider->getThumbnail($this->imgPath, $this->maxWidth, $this->maxHeight, $this->scalingUp, $this->rootView); + $preview = $provider->getThumbnail($this->imgPath, $this->maxWidth, $this->maxHeight, $this->scalingUp); $this->assertNotFalse($preview); $this->assertTrue($preview->valid()); diff --git a/tests/lib/PreviewTest.php b/tests/lib/PreviewTest.php index f4dcff398f0b..c60372eddc3d 100644 --- a/tests/lib/PreviewTest.php +++ b/tests/lib/PreviewTest.php @@ -22,11 +22,11 @@ namespace Test; -use OC\Files\FileInfo; use OC\Files\Filesystem; use OC\Files\Storage\Temporary; use OC\Files\View; use OC\Preview; +use OC\PreviewManager; use Test\Traits\MountProviderTrait; use Test\Traits\UserTrait; @@ -36,6 +36,7 @@ * @group DB * * @package Test + * @requires extension imagick */ class PreviewTest extends TestCase { use UserTrait; @@ -107,6 +108,12 @@ protected function setUp() { \OC::$server->getConfig() ->setSystemValue('enabledPreviewProviders', $providers); + //re-initialize the preview manager due to config change above + unset(\OC::$server['PreviewManager']); + \OC::$server->registerService('PreviewManager', function ($c) { + return new PreviewManager($c->getConfig()); + }); + // Sample is 1680x1050 JPEG $this->prepareSample('testimage.jpg', 1680, 1050); // Sample is 2400x1707 EPS @@ -135,7 +142,8 @@ public function testIsPreviewDeleted() { $x = 50; $y = 50; - $preview = new Preview(self::TEST_PREVIEW_USER1, 'files/', 'test.txt', $x, $y); + $file = \OC::$server->getUserFolder(self::TEST_PREVIEW_USER1)->get('test.txt'); + $preview = new Preview(self::TEST_PREVIEW_USER1, 'files/', $file, $x, $y); $preview->getPreview(); $fileInfo = $this->rootView->getFileInfo($sampleFile); @@ -166,7 +174,8 @@ public function testAreAllPreviewsDeleted() { $x = 50; $y = 50; - $preview = new Preview(self::TEST_PREVIEW_USER1, 'files/', 'test.txt', $x, $y); + $file = \OC::$server->getUserFolder(self::TEST_PREVIEW_USER1)->get('test.txt'); + $preview = new Preview(self::TEST_PREVIEW_USER1, 'files/', $file, $x, $y); $preview->getPreview(); $fileInfo = $this->rootView->getFileInfo($sampleFile); @@ -205,8 +214,9 @@ public function testIsTransparent($extension, $data, $expectedResult) { $sample = '/' . self::TEST_PREVIEW_USER1 . '/files/test.' . $extension; $this->rootView->file_put_contents($sample, $data); + $file = \OC::$server->getUserFolder(self::TEST_PREVIEW_USER1)->get("test.$extension"); $preview = new Preview( - self::TEST_PREVIEW_USER1, 'files/', 'test.' . $extension, $x, + self::TEST_PREVIEW_USER1, 'files/', $file, $x, $y ); $image = $preview->getPreview(); @@ -234,8 +244,8 @@ public function testUnsupportedPreviewsReturnEmptyObject() { $imgPath = '/' . self::TEST_PREVIEW_USER1 . '/files/testimage.odt'; $this->rootView->file_put_contents($imgPath, $imgData); - $preview = - new Preview(self::TEST_PREVIEW_USER1, 'files/', 'testimage.odt', $width, $height); + $file = \OC::$server->getUserFolder(self::TEST_PREVIEW_USER1)->get('testimage.odt'); + $preview = new Preview(self::TEST_PREVIEW_USER1, 'files/', $file, $width, $height); $preview->getPreview(); $image = $preview->getPreview(); @@ -401,7 +411,7 @@ public function testSecondPreviewsGetCachedMax( $preview = $this->createPreview($previewWidth, $previewHeight); // A cache query should return the thumbnail of max dimension - $isCached = $preview->isCached($sampleFileId); + $isCached = $preview->isCached(); $cachedMaxPreview = $this->buildCachePath( $sampleFileId, $this->maxPreviewWidth, $this->maxPreviewHeight, false, '-max' ); @@ -418,14 +428,13 @@ public function testMaxPreviewCannotBeDeleted() { $this->keepAspect = true; $this->getSample(0); - $fileId = $this->sampleFileId; //Creates the Max preview which we will try to delete $preview = $this->createMaxPreview(); // We try to deleted the preview $preview->deletePreview(); - $this->assertNotFalse($preview->isCached($fileId)); + $this->assertNotFalse($preview->isCached()); $preview->deleteAllPreviews(); } @@ -545,7 +554,7 @@ public function testIsBiggerWithAspectRatioCached( // Small thumbnails are always cropped $this->keepAspect = false; // Smaller previews should be based on the previous, larger preview, with the correct aspect ratio - $this->createThumbnailFromBiggerCachedPreview($fileId, 32, 32); + $this->createThumbnailFromBiggerCachedPreview(32, 32); // 2nd cache query should indicate that we have a cached copy of the exact dimension $this->getCachedSmallThumbnail($fileId, 32, 32); @@ -565,8 +574,9 @@ public function testIsBiggerWithAspectRatioCached( * @return Preview */ private function createPreview($width, $height) { + $file = \OC::$server->getUserFolder(self::TEST_PREVIEW_USER1)->get($this->sampleFilename); $preview = new Preview( - self::TEST_PREVIEW_USER1, 'files/', $this->sampleFilename, $width, + self::TEST_PREVIEW_USER1, 'files/', $file, $width, $height ); @@ -649,15 +659,15 @@ private function getSmallerThanMaxPreview($fileId, $previewWidth, $previewHeight // And it should be cached $this->checkCache($fileId, $limitedPreviewWidth, $limitedPreviewHeight); - $this->cachedBigger[] = $preview->isCached($fileId); + $this->cachedBigger[] = $preview->isCached(); } - private function createThumbnailFromBiggerCachedPreview($fileId, $width, $height) { + private function createThumbnailFromBiggerCachedPreview($width, $height) { $preview = $this->createPreview($width, $height); // A cache query should return a thumbnail of slightly larger dimensions // and with the proper aspect ratio - $isCached = $preview->isCached($fileId); + $isCached = $preview->isCached(); $expectedCachedBigger = $this->getExpectedCachedBigger(); $this->assertSame($expectedCachedBigger, $isCached); @@ -718,7 +728,7 @@ private function getExpectedCachedBigger() { private function getCachedSmallThumbnail($fileId, $width, $height) { $preview = $this->createPreview($width, $height); - $isCached = $preview->isCached($fileId); + $isCached = $preview->isCached(); $thumbCacheFile = $this->buildCachePath($fileId, $width, $height); $this->assertSame($thumbCacheFile, $isCached, "$thumbCacheFile \n"); @@ -884,8 +894,9 @@ public function testKeepAspectRatio() { $originalHeight = 1050; $originalAspectRation = $originalWidth / $originalHeight; + $file = \OC::$server->getUserFolder(self::TEST_PREVIEW_USER1)->get('testimage.jpg'); $preview = new Preview( - self::TEST_PREVIEW_USER1, 'files/', 'testimage.jpg', + self::TEST_PREVIEW_USER1, 'files/', $file, 150, 150 ); @@ -904,8 +915,9 @@ public function testKeepAspectRatioCover() { $originalHeight = 1050; $originalAspectRation = $originalWidth / $originalHeight; + $file = \OC::$server->getUserFolder(self::TEST_PREVIEW_USER1)->get('testimage.jpg'); $preview = new Preview( - self::TEST_PREVIEW_USER1, 'files/', 'testimage.jpg', + self::TEST_PREVIEW_USER1, 'files/', $file, 150, 150 ); @@ -920,13 +932,6 @@ public function testKeepAspectRatioCover() { $this->assertGreaterThanOrEqual(150, $image->height()); } - public function testSetFileWithInfo() { - $info = new FileInfo('/foo', null, '/foo', ['mimetype' => 'foo/bar'], null); - $preview = new Preview(); - $preview->setFile('/foo', $info); - $this->assertEquals($info, $this->invokePrivate($preview, 'getFileInfo')); - } - public function testIsCached() { $sourceFile = __DIR__ . '/../data/testimage.png'; $userId = $this->getUniqueID(); @@ -941,16 +946,16 @@ public function testIsCached() { $preview = new Preview($userId, 'files'); $view = new View('/' . $userId . '/files'); $view->file_put_contents('test.png', file_get_contents($sourceFile)); - $info = $view->getFileInfo('test.png'); - $preview->setFile('test.png', $info); + $file = \OC::$server->getUserFolder($userId)->get('test.png'); + $preview->setFile($file); $preview->setMaxX(64); $preview->setMaxY(64); - $this->assertFalse($preview->isCached($info->getId())); + $this->assertFalse($preview->isCached()); $preview->getPreview(); - $this->assertEquals('thumbnails/' . $info->getId() . '/64-64.png', $preview->isCached($info->getId())); + $this->assertEquals('thumbnails/' . $file->getId() . '/64-64.png', $preview->isCached()); } }