From b4ad2aa4667afe29c6ede46b16fe6c05434d793c Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Mon, 24 Aug 2015 11:32:05 +0200 Subject: [PATCH 1/5] Use the object ID to generate the preview --- controller/activities.php | 81 +++++++++++++++++++++++++++------------ lib/grouphelper.php | 13 +++---- 2 files changed, 62 insertions(+), 32 deletions(-) diff --git a/controller/activities.php b/controller/activities.php index d7fa6611..08a35c70 100644 --- a/controller/activities.php +++ b/controller/activities.php @@ -152,22 +152,22 @@ public function fetch($page, $filter = 'all', $objecttype = '', $objectid = 0) { } $activity['previews'] = []; - if (!empty($activity['files'])) { - foreach ($activity['files'] as $file) { - if ($file === '') { + if ($activity['object_type'] === 'files' && !empty($activity['object_ids'])) { + foreach ($activity['files'] as $objectId => $objectName) { + if (((int) $objectId) === 0 || $objectName === '') { // No file, no preview continue; } - $activity['previews'][] = $this->getPreview($activity, $file); + $activity['previews'][] = $this->getPreview($activity, (int) $objectId, $objectName); if (sizeof($activity['previews']) >= self::MAX_NUM_THUMBNAILS) { // Don't want to clutter the page, so we stop after a few thumbnails break; } } - } else if ($activity['file'] !== '') { - $activity['previews'][] = $this->getPreview($activity, $activity['file']); + } else if ($activity['object_type'] === 'files' && $activity['object_id']) { + $activity['previews'][] = $this->getPreview($activity, (int) $activity['object_id'], $activity['file']); } $preparedActivities[] = $activity; @@ -178,48 +178,81 @@ public function fetch($page, $filter = 'all', $objecttype = '', $objectid = 0) { /** * @param array $activity - * @param string $file + * @param int $fileId + * @param string $filePath * @return array */ - protected function getPreview(array $activity, $file) { + protected function getPreview(array $activity, $fileId, $filePath) { $this->view->chroot('/' . $activity['affecteduser'] . '/files'); - $exist = $this->view->file_exists($file); - $is_dir = $this->view->is_dir($file); + $path = $this->view->getPath($fileId); + + if ($path === null || $path === '' || !$this->view->file_exists($path)) { + return $this->getPreviewFromPath($filePath); + } + + $is_dir = $this->view->is_dir($path); $preview = [ - 'link' => $this->getPreviewLink($file, $is_dir), + 'link' => $this->getPreviewLink($path, $is_dir), 'source' => '', 'isMimeTypeIcon' => true, ]; // show a preview image if the file still exists if ($is_dir) { - $mimeTypeIcon = Template::mimetype_icon('dir'); - if (substr($mimeTypeIcon, -4) === '.png') { - $mimeTypeIcon = substr($mimeTypeIcon, 0, -4) . '.svg'; - } - $preview['source'] = $mimeTypeIcon; + $preview['source'] = $this->getPreviewPathFromMimeType('dir'); } else { - $mimeType = Files::getMimeType($file); - if (!$is_dir && $mimeType && $this->preview->isMimeSupported($mimeType) && $exist) { + $fileInfo = $this->view->getFileInfo($path); + $mimeType = $fileInfo->getMimetype(); + if (!$is_dir && $mimeType && $this->preview->isMimeSupported($mimeType)) { $preview['isMimeTypeIcon'] = false; $preview['source'] = $this->urlGenerator->linkToRoute('core_ajax_preview', [ - 'file' => $file, + 'file' => $path, + 'c' => $this->view->getETag($path), 'x' => 150, 'y' => 150, ]); } else { - $mimeTypeIcon = Template::mimetype_icon($mimeType); - if (substr($mimeTypeIcon, -4) === '.png') { - $mimeTypeIcon = substr($mimeTypeIcon, 0, -4) . '.svg'; - } - $preview['source'] = $mimeTypeIcon; + $preview['source'] = $this->getPreviewPathFromMimeType($mimeType); } } return $preview; } + /** + * @param string $filePath + * @return array + */ + protected function getPreviewFromPath($filePath) { + $mimeType = Files::getMimeType($filePath); + $mimeTypeIcon = Template::mimetype_icon($mimeType); + if (substr($mimeTypeIcon, -4) === '.png') { + $mimeTypeIcon = substr($mimeTypeIcon, 0, -4) . '.svg'; + } + + $preview = [ + 'link' => $this->getPreviewLink($filePath, false), + 'source' => $mimeTypeIcon, + 'isMimeTypeIcon' => true, + ]; + + return $preview; + } + + /** + * @param string $mimeType + * @return string + */ + protected function getPreviewPathFromMimeType($mimeType) { + $mimeTypeIcon = Template::mimetype_icon($mimeType); + if (substr($mimeTypeIcon, -4) === '.png') { + $mimeTypeIcon = substr($mimeTypeIcon, 0, -4) . '.svg'; + } + + return $mimeTypeIcon; + } + /** * @param string $path * @param bool $isDir diff --git a/lib/grouphelper.php b/lib/grouphelper.php index e61495dd..7bdcf23c 100644 --- a/lib/grouphelper.php +++ b/lib/grouphelper.php @@ -108,20 +108,17 @@ public function addActivity($activity) { $this->openGroup['subjectparams_array'][$parameter] = array($this->openGroup['subjectparams_array'][$parameter]); } if (!isset($this->openGroup['activity_ids'])) { - $this->openGroup['activity_ids'] = array((int) $this->openGroup['activity_id']); - $this->openGroup['files'] = array((string) $this->openGroup['file']); - $this->openGroup['object_ids'] = array((int) $this->openGroup['object_id']); + $this->openGroup['activity_ids'] = [(int) $this->openGroup['activity_id']]; + $this->openGroup['files'] = [ + (int) $this->openGroup['object_id'] => (string) $this->openGroup['file'] + ]; } $this->openGroup['subjectparams_array'][$parameter][] = $activity['subjectparams_array'][$parameter]; $this->openGroup['subjectparams_array'][$parameter] = array_unique($this->openGroup['subjectparams_array'][$parameter]); $this->openGroup['activity_ids'][] = (int) $activity['activity_id']; - $objectId = (int) $activity['object_id']; - if (!in_array($objectId, $this->openGroup['object_ids'])) { - $this->openGroup['files'][] = (string) $activity['file']; - $this->openGroup['object_ids'][] = $objectId; - } + $this->openGroup['files'][(int) $activity['object_id']] = (string) $activity['file']; } } else { $this->closeOpenGroup(); From 6e4d97d068e97e41b8fab5199b5cb3a5059fee72 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 28 Aug 2015 23:34:34 +0200 Subject: [PATCH 2/5] Fix unit tests, still missing coverage for the preview methods thou --- controller/activities.php | 2 +- tests/controller/activitiestest.php | 140 ++++++++++++++++++++-------- tests/grouphelpertest.php | 17 ++-- 3 files changed, 112 insertions(+), 47 deletions(-) diff --git a/controller/activities.php b/controller/activities.php index 08a35c70..c81e669b 100644 --- a/controller/activities.php +++ b/controller/activities.php @@ -152,7 +152,7 @@ public function fetch($page, $filter = 'all', $objecttype = '', $objectid = 0) { } $activity['previews'] = []; - if ($activity['object_type'] === 'files' && !empty($activity['object_ids'])) { + if ($activity['object_type'] === 'files' && !empty($activity['files'])) { foreach ($activity['files'] as $objectId => $objectName) { if (((int) $objectId) === 0 || $objectName === '') { // No file, no preview diff --git a/tests/controller/activitiestest.php b/tests/controller/activitiestest.php index f7a5bdcc..2b158d56 100644 --- a/tests/controller/activitiestest.php +++ b/tests/controller/activitiestest.php @@ -81,19 +81,42 @@ protected function setUp() { $this->request = $this->getMock('OCP\IRequest'); - $this->controller = new Activities( - 'activity', - $this->request, - $this->data, - $this->helper, - $this->navigation, - $this->userSettings, - $this->dateTimeFormatter, - $this->preview, - $this->urlGenerator, - $this->view, - 'test' - ); + $this->controller = $this->getController(); + } + + protected function getController(array $methods = []) { + if (empty($methods)) { + return new Activities( + 'activity', + $this->request, + $this->data, + $this->helper, + $this->navigation, + $this->userSettings, + $this->dateTimeFormatter, + $this->preview, + $this->urlGenerator, + $this->view, + 'test' + ); + } else { + return $this->getMockBuilder('OCA\Activity\Controller\Activities') + ->setConstructorArgs([ + 'activity', + $this->request, + $this->data, + $this->helper, + $this->navigation, + $this->userSettings, + $this->dateTimeFormatter, + $this->preview, + $this->urlGenerator, + $this->view, + 'test', + ]) + ->setMethods($methods) + ->getMock(); + } } public function testShowList() { @@ -113,12 +136,6 @@ public function testShowList() { } public function dataFetch() { - $folderPreview = [ - 'link' => 'linkToStub', - 'source' => '/core/img/filetypes/folder.svg', - 'isMimeTypeIcon' => true, - ]; - $timestamp = time(); return [ [ @@ -153,6 +170,8 @@ public function dataFetch() { 'full' => 'message.markup.full', ], ], + 'object_type' => 'files', + 'object_id' => 21, ], ], [ @@ -187,12 +206,10 @@ public function dataFetch() { 'relativeDateTimestamp' => 'seconds ago', 'readableDateTimestamp' => (string) $timestamp, + 'object_type' => 'files', + 'object_id' => 21, 'previews' => [ - [ - 'link' => 'linkToStub', - 'source' => 'linkToRouteStub', - 'isMimeTypeIcon' => false, - ], + ['preview'], ], ], ], @@ -225,6 +242,8 @@ public function dataFetch() { 'full' => 'message.markup.full', ], ], + 'object_type' => 'files', + 'object_id' => 21, ], ], [ @@ -259,12 +278,10 @@ public function dataFetch() { 'relativeDateTimestamp' => 'seconds ago', 'readableDateTimestamp' => (string) $timestamp, + 'object_type' => 'files', + 'object_id' => 21, 'previews' => [ - [ - 'link' => 'linkToStub', - 'source' => '/core/img/filetypes/audio.svg', - 'isMimeTypeIcon' => true, - ], + ['preview'], ], ], ], @@ -297,6 +314,8 @@ public function dataFetch() { 'full' => 'message.markup.full', ], ], + 'object_type' => 'files', + 'object_id' => 21, ], ], [ @@ -331,7 +350,11 @@ public function dataFetch() { 'relativeDateTimestamp' => 'seconds ago', 'readableDateTimestamp' => (string) $timestamp, - 'previews' => [$folderPreview], + 'object_type' => 'files', + 'object_id' => 21, + 'previews' => [ + ['preview'] + ], ], ], ], @@ -344,7 +367,17 @@ public function dataFetch() { 'app' => 'files', 'link' => 'http://localhost', 'file' => '/directory', - 'files' => ['/directory', '', '/directory', '/directory', '/directory', '/directory', '/directory', '/directory', '/directory'], + 'files' => [ + 21 => '/directory', + 20 => '', + 19 => '/directory', + 18 => '/directory', + 17 => '/directory', + 16 => '/directory', + 15 => '/directory', + 14 => '/directory', + 13 => '/directory', + ], 'typeicon' => '', 'subject' => 'subject', 'subjectformatted' => [ @@ -364,6 +397,8 @@ public function dataFetch() { 'full' => 'message.markup.full', ], ], + 'object_type' => 'files', + 'object_id' => 21, ], ], [ @@ -374,7 +409,17 @@ public function dataFetch() { 'app' => 'files', 'link' => '', 'file' => '/directory', - 'files' => ['/directory', '', '/directory', '/directory', '/directory', '/directory', '/directory', '/directory', '/directory'], + 'files' => [ + 21 => '/directory', + 20 => '', + 19 => '/directory', + 18 => '/directory', + 17 => '/directory', + 16 => '/directory', + 15 => '/directory', + 14 => '/directory', + 13 => '/directory', + ], 'typeicon' => '', 'subject' => 'subject', 'subjectformatted' => [ @@ -399,14 +444,16 @@ public function dataFetch() { 'relativeDateTimestamp' => 'seconds ago', 'readableDateTimestamp' => (string) $timestamp, + 'object_type' => 'files', + 'object_id' => 21, 'previews' => [ - $folderPreview, - $folderPreview, - $folderPreview, - $folderPreview, - $folderPreview, - $folderPreview, - $folderPreview, + ['preview'], + ['preview'], + ['preview'], + ['preview'], + ['preview'], + ['preview'], + ['preview'], ], ], ], @@ -439,6 +486,12 @@ public function testFetch($readData, $expected) { ->method('linkToRoute') ->willReturn('linkToRouteStub'); + $this->view->expects($this->any()) + ->method('getPath') + ->willReturnMap([ + [21, '/file.txt'], + [42, null], + ]); $this->view->expects($this->any()) ->method('is_dir') ->willReturnMap([ @@ -461,8 +514,15 @@ public function testFetch($readData, $expected) { ['audio/mpeg', false], ]); + $controller = $this->getController([ + 'getPreview' + ]); + $controller->expects($this->any()) + ->method('getPreview') + ->willReturn(['preview']); + /** @var \OCP\AppFramework\Http\JSONResponse $response */ - $response = $this->controller->fetch(1); + $response = $controller->fetch(1); $this->assertInstanceOf('\OCP\AppFramework\Http\JSONResponse', $response, 'Asserting type of return is \OCP\AppFramework\Http\TemplateResponse'); $this->assertEquals($expected, $response->getData()); diff --git a/tests/grouphelpertest.php b/tests/grouphelpertest.php index 6b5b5a63..85e538fb 100644 --- a/tests/grouphelpertest.php +++ b/tests/grouphelpertest.php @@ -165,8 +165,10 @@ public function groupHelperData() { 'message' => '', 'messageparams' => array(), 'file' => 'testing/file2.txt', - 'files' => ['testing/file2.txt', 'testing/file1.txt'], - 'object_ids' => [42, 21], + 'files' => [ + 42 => 'testing/file2.txt', + 21 => 'testing/file1.txt', + ], 'typeicon' => 'icon-type1', 'object_type' => 'files', 'object_id' => 42, @@ -222,8 +224,9 @@ public function groupHelperData() { 'message' => '', 'messageparams' => array(), 'file' => 'testing/file2.txt', - 'files' => ['testing/file2.txt'], - 'object_ids' => [42], + 'files' => [ + 42 => 'testing/file2.txt' + ], 'typeicon' => 'icon-type1', 'object_type' => 'files', 'object_id' => 42, @@ -295,8 +298,10 @@ public function groupHelperData() { 'message' => '', 'messageparams' => array(), 'file' => 'testing/file3.txt', - 'files' => ['testing/file3.txt', 'testing/file2.txt'], - 'object_ids' => [1337, 42], + 'files' => [ + 1337 => 'testing/file3.txt', + 42 => 'testing/file2.txt', + ], 'typeicon' => 'icon-type1', 'object_type' => 'files', 'object_id' => 1337, From f34548668b1c0cdcacbc8516be4924fccb9809cd Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Sat, 29 Aug 2015 01:27:05 +0200 Subject: [PATCH 3/5] Use IMimeTypeDetector --- appinfo/application.php | 1 + controller/activities.php | 28 ++++++----- tests/controller/activitiestest.php | 75 +++++++++++++++++++++++++++++ 3 files changed, 91 insertions(+), 13 deletions(-) diff --git a/appinfo/application.php b/appinfo/application.php index 075a3573..15aec47d 100644 --- a/appinfo/application.php +++ b/appinfo/application.php @@ -206,6 +206,7 @@ public function __construct (array $urlParams = array()) { $server->getDateTimeFormatter(), $server->getPreviewManager(), $server->getURLGenerator(), + $server->getMimeTypeDetector(), new View(''), $c->query('CurrentUID') ); diff --git a/controller/activities.php b/controller/activities.php index c81e669b..9ab73960 100644 --- a/controller/activities.php +++ b/controller/activities.php @@ -32,6 +32,7 @@ use OCP\AppFramework\Http\JSONResponse; use OCP\AppFramework\Http\TemplateResponse; use OCP\Files; +use OCP\Files\IMimeTypeDetector; use OCP\IDateTimeFormatter; use OCP\IPreview; use OCP\IRequest; @@ -64,6 +65,9 @@ class Activities extends Controller { /** @var IURLGenerator */ protected $urlGenerator; + /** @var mimeTypeDetector */ + protected $mimeTypeDetector; + /** @var View */ protected $view; @@ -82,6 +86,7 @@ class Activities extends Controller { * @param IDateTimeFormatter $dateTimeFormatter * @param IPreview $preview * @param IURLGenerator $urlGenerator + * @param IMimeTypeDetector $mimeTypeDetector * @param View $view * @param string $user */ @@ -94,6 +99,7 @@ public function __construct($appName, IDateTimeFormatter $dateTimeFormatter, IPreview $preview, IURLGenerator $urlGenerator, + IMimeTypeDetector $mimeTypeDetector, View $view, $user) { parent::__construct($appName, $request); @@ -104,6 +110,7 @@ public function __construct($appName, $this->dateTimeFormatter = $dateTimeFormatter; $this->preview = $preview; $this->urlGenerator = $urlGenerator; + $this->mimeTypeDetector = $mimeTypeDetector; $this->view = $view; $this->user = $user; } @@ -159,7 +166,7 @@ public function fetch($page, $filter = 'all', $objecttype = '', $objectid = 0) { continue; } - $activity['previews'][] = $this->getPreview($activity, (int) $objectId, $objectName); + $activity['previews'][] = $this->getPreview($activity['affecteduser'], (int) $objectId, $objectName); if (sizeof($activity['previews']) >= self::MAX_NUM_THUMBNAILS) { // Don't want to clutter the page, so we stop after a few thumbnails @@ -167,7 +174,7 @@ public function fetch($page, $filter = 'all', $objecttype = '', $objectid = 0) { } } } else if ($activity['object_type'] === 'files' && $activity['object_id']) { - $activity['previews'][] = $this->getPreview($activity, (int) $activity['object_id'], $activity['file']); + $activity['previews'][] = $this->getPreview($activity['affecteduser'], (int) $activity['object_id'], $activity['file']); } $preparedActivities[] = $activity; @@ -177,13 +184,13 @@ public function fetch($page, $filter = 'all', $objecttype = '', $objectid = 0) { } /** - * @param array $activity + * @param string $owner * @param int $fileId * @param string $filePath * @return array */ - protected function getPreview(array $activity, $fileId, $filePath) { - $this->view->chroot('/' . $activity['affecteduser'] . '/files'); + protected function getPreview($owner, $fileId, $filePath) { + $this->view->chroot('/' . $owner . '/files'); $path = $this->view->getPath($fileId); if ($path === null || $path === '' || !$this->view->file_exists($path)) { @@ -225,15 +232,10 @@ protected function getPreview(array $activity, $fileId, $filePath) { * @return array */ protected function getPreviewFromPath($filePath) { - $mimeType = Files::getMimeType($filePath); - $mimeTypeIcon = Template::mimetype_icon($mimeType); - if (substr($mimeTypeIcon, -4) === '.png') { - $mimeTypeIcon = substr($mimeTypeIcon, 0, -4) . '.svg'; - } - + $mimeType = $this->mimeTypeDetector->detectPath($filePath); $preview = [ 'link' => $this->getPreviewLink($filePath, false), - 'source' => $mimeTypeIcon, + 'source' => $this->getPreviewPathFromMimeType($mimeType), 'isMimeTypeIcon' => true, ]; @@ -245,7 +247,7 @@ protected function getPreviewFromPath($filePath) { * @return string */ protected function getPreviewPathFromMimeType($mimeType) { - $mimeTypeIcon = Template::mimetype_icon($mimeType); + $mimeTypeIcon = $this->mimeTypeDetector->mimeTypeIcon($mimeType); if (substr($mimeTypeIcon, -4) === '.png') { $mimeTypeIcon = substr($mimeTypeIcon, 0, -4) . '.svg'; } diff --git a/tests/controller/activitiestest.php b/tests/controller/activitiestest.php index 2b158d56..179d2036 100644 --- a/tests/controller/activitiestest.php +++ b/tests/controller/activitiestest.php @@ -42,6 +42,9 @@ class ActivitiesTest extends TestCase { /** @var \PHPUnit_Framework_MockObject_MockObject */ protected $urlGenerator; + /** @var \PHPUnit_Framework_MockObject_MockObject */ + protected $mimeTypeDetector; + /** @var \PHPUnit_Framework_MockObject_MockObject */ protected $view; @@ -75,6 +78,9 @@ protected function setUp() { $this->urlGenerator = $this->getMockBuilder('OCP\IURLGenerator') ->disableOriginalConstructor() ->getMock(); + $this->mimeTypeDetector = $this->getMockBuilder('OCP\Files\IMimeTypeDetector') + ->disableOriginalConstructor() + ->getMock(); $this->view = $this->getMockBuilder('OC\Files\View') ->disableOriginalConstructor() ->getMock(); @@ -96,6 +102,7 @@ protected function getController(array $methods = []) { $this->dateTimeFormatter, $this->preview, $this->urlGenerator, + $this->mimeTypeDetector, $this->view, 'test' ); @@ -111,6 +118,7 @@ protected function getController(array $methods = []) { $this->dateTimeFormatter, $this->preview, $this->urlGenerator, + $this->mimeTypeDetector, $this->view, 'test', ]) @@ -528,6 +536,73 @@ public function testFetch($readData, $expected) { $this->assertEquals($expected, $response->getData()); } + public function dataGetPreviewFromPath() { + return [ + ['dir', 'dir', '/core/img/filetypes/folder.svg'], + ['test.txt', 'text/plain', '/core/img/filetypes/text.svg'], + ['test.mp3', 'audio/mpeg', '/core/img/filetypes/audio.svg'], + ]; + } + + /** + * @dataProvider dataGetPreviewFromPath + * @param string $filePath + * @param string $mimeType + * @param string $expected + */ + public function testGetPreviewFromPath($filePath, $mimeType) { + $controller = $this->getController([ + 'getPreviewPathFromMimeType', + 'getPreviewLink', + ]); + + $controller->expects($this->once()) + ->method('getPreviewPathFromMimeType') + ->with($mimeType) + ->willReturn('mime-type-icon'); + $controller->expects($this->once()) + ->method('getPreviewLink') + ->with($filePath, false) + ->willReturn('target-link'); + $this->mimeTypeDetector->expects($this->once()) + ->method('detectPath') + ->willReturn($mimeType); + + $this->assertSame( + [ + 'link' => 'target-link', + 'source' => 'mime-type-icon', + 'isMimeTypeIcon' => true, + ], + $this->invokePrivate($controller, 'getPreviewFromPath', [$filePath]) + ); + } + + public function dataGetPreviewPathFromMimeType() { + return [ + ['dir', '/core/img/filetypes/folder.png', '/core/img/filetypes/folder.svg'], + ['text/plain', '/core/img/filetypes/text.svg', '/core/img/filetypes/text.svg'], + ['text/plain', '/core/img/filetypes/text.jpg', '/core/img/filetypes/text.jpg'], + ]; + } + + /** + * @dataProvider dataGetPreviewPathFromMimeType + * @param string $mimeType + * @param string $expected + */ + public function testGetPreviewPathFromMimeType($mimeType, $icon, $expected) { + $this->mimeTypeDetector->expects($this->once()) + ->method('mimeTypeIcon') + ->with($mimeType) + ->willReturn($icon); + + $this->assertSame( + $expected, + $this->invokePrivate($this->controller, 'getPreviewPathFromMimeType', [$mimeType]) + ); + } + public function dataGetPreviewLink() { return [ ['/folder', true, ['dir' => '/folder']], From 90ce13c557c7dc79f247d82530a1250a78422d19 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Sat, 29 Aug 2015 16:05:38 +0200 Subject: [PATCH 4/5] Add tests for getPreview() --- controller/activities.php | 2 +- tests/controller/activitiestest.php | 140 ++++++++++++++++++++++++++++ 2 files changed, 141 insertions(+), 1 deletion(-) diff --git a/controller/activities.php b/controller/activities.php index 9ab73960..a383c36c 100644 --- a/controller/activities.php +++ b/controller/activities.php @@ -211,7 +211,7 @@ protected function getPreview($owner, $fileId, $filePath) { } else { $fileInfo = $this->view->getFileInfo($path); $mimeType = $fileInfo->getMimetype(); - if (!$is_dir && $mimeType && $this->preview->isMimeSupported($mimeType)) { + if ($mimeType && $this->preview->isMimeSupported($mimeType)) { $preview['isMimeTypeIcon'] = false; $preview['source'] = $this->urlGenerator->linkToRoute('core_ajax_preview', [ 'file' => $path, diff --git a/tests/controller/activitiestest.php b/tests/controller/activitiestest.php index 179d2036..0e067af5 100644 --- a/tests/controller/activitiestest.php +++ b/tests/controller/activitiestest.php @@ -536,6 +536,146 @@ public function testFetch($readData, $expected) { $this->assertEquals($expected, $response->getData()); } + public function dataGetPreviewInvalidPaths() { + return [ + ['author', 42, '/path', null, null], + ['author', 42, '/path', '', null], + ['author', 42, '/path', '/currentPath', false], + ]; + } + + /** + * @dataProvider dataGetPreviewInvalidPaths + * + * @param string $author + * @param int $fileId + * @param string $path + * @param string $returnedPath + * @param null|bool $exists + */ + public function testGetPreviewInvalidPaths($author, $fileId, $path, $returnedPath, $exists) { + $this->view->expects($this->once()) + ->method('chroot') + ->with('/' . $author . '/files'); + $this->view->expects($this->once()) + ->method('getPath') + ->with($fileId) + ->willReturn($returnedPath); + if ($exists === null) { + $this->view->expects($this->never()) + ->method('file_exists'); + } else { + $this->view->expects($this->once()) + ->method('file_exists') + ->willReturn($exists); + } + + $controller = $this->getController([ + 'getPreviewFromPath' + ]); + $controller->expects($this->any()) + ->method('getPreviewFromPath') + ->with($path) + ->willReturn(['getPreviewFromPath']); + + $this->assertSame(['getPreviewFromPath'], $this->invokePrivate($controller, 'getPreview', [$author, $fileId, $path])); + } + + public function dataGetPreview() { + return [ + ['author', 42, '/path', '/currentPath', true, false, '/preview/dir', true], + ['author', 42, '/file.txt', '/currentFile.txt', false, false, '/preview/mpeg', true], + ['author', 42, '/file.txt', '/currentFile.txt', false, true, '/preview/currentFile.txt', false], + ]; + } + + /** + * @dataProvider dataGetPreview + * + * @param string $author + * @param int $fileId + * @param string $path + * @param string $returnedPath + * @param bool $isDir + * @param bool $isMimeSup + * @param string $source + * @param bool $isMimeTypeIcon + */ + public function testGetPreview($author, $fileId, $path, $returnedPath, $isDir, $isMimeSup, $source, $isMimeTypeIcon) { + + $controller = $this->getController([ + 'getPreviewLink', + 'getPreviewPathFromMimeType', + ]); + + $this->view->expects($this->once()) + ->method('chroot') + ->with('/' . $author . '/files'); + $this->view->expects($this->once()) + ->method('getPath') + ->with($fileId) + ->willReturn($returnedPath); + $this->view->expects($this->once()) + ->method('file_exists') + ->with($returnedPath) + ->willReturn(true); + $this->view->expects($this->once()) + ->method('is_dir') + ->with($returnedPath) + ->willReturn($isDir); + + $controller->expects($this->once()) + ->method('getPreviewLink') + ->with($returnedPath, $isDir) + ->willReturnCallback(function($path) { + return '/preview' . $path; + }); + + if ($isDir) { + $controller->expects($this->once()) + ->method('getPreviewPathFromMimeType') + ->with('dir') + ->willReturn('/preview/dir'); + } else { + $fileInfo = $this->getMockBuilder('OCP\Files\FileInfo') + ->disableOriginalConstructor() + ->getMock(); + $fileInfo->expects($this->once()) + ->method('getMimetype') + ->willReturn('audio/mp3'); + + $this->view->expects($this->once()) + ->method('getFileInfo') + ->with($returnedPath) + ->willReturn($fileInfo); + + $this->preview->expects($this->once()) + ->method('isMimeSupported') + ->with('audio/mp3') + ->willReturn($isMimeSup); + + if (!$isMimeSup) { + $controller->expects($this->once()) + ->method('getPreviewPathFromMimeType') + ->with('audio/mp3') + ->willReturn('/preview/mpeg'); + } else { + $this->urlGenerator->expects($this->once()) + ->method('linkToRoute') + ->with('core_ajax_preview', $this->anything()) + ->willReturnCallback(function() use ($returnedPath) { + return '/preview' . $returnedPath; + }); + } + } + + $this->assertSame([ + 'link' => '/preview' . $returnedPath, + 'source' => $source, + 'isMimeTypeIcon' => $isMimeTypeIcon, + ], $this->invokePrivate($controller, 'getPreview', [$author, $fileId, $path])); + } + public function dataGetPreviewFromPath() { return [ ['dir', 'dir', '/core/img/filetypes/folder.svg'], From 7f892b94de98aa0903a920d42b48bc0b0fb3eab7 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Sat, 29 Aug 2015 17:30:10 +0200 Subject: [PATCH 5/5] Check if the preview isAvailable instead of supported ;) --- controller/activities.php | 5 ++--- tests/controller/activitiestest.php | 11 ++++++----- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/controller/activities.php b/controller/activities.php index a383c36c..2a293b9c 100644 --- a/controller/activities.php +++ b/controller/activities.php @@ -210,8 +210,7 @@ protected function getPreview($owner, $fileId, $filePath) { $preview['source'] = $this->getPreviewPathFromMimeType('dir'); } else { $fileInfo = $this->view->getFileInfo($path); - $mimeType = $fileInfo->getMimetype(); - if ($mimeType && $this->preview->isMimeSupported($mimeType)) { + if ($this->preview->isAvailable($fileInfo)) { $preview['isMimeTypeIcon'] = false; $preview['source'] = $this->urlGenerator->linkToRoute('core_ajax_preview', [ 'file' => $path, @@ -220,7 +219,7 @@ protected function getPreview($owner, $fileId, $filePath) { 'y' => 150, ]); } else { - $preview['source'] = $this->getPreviewPathFromMimeType($mimeType); + $preview['source'] = $this->getPreviewPathFromMimeType($fileInfo->getMimetype()); } } diff --git a/tests/controller/activitiestest.php b/tests/controller/activitiestest.php index 0e067af5..01339492 100644 --- a/tests/controller/activitiestest.php +++ b/tests/controller/activitiestest.php @@ -640,9 +640,6 @@ public function testGetPreview($author, $fileId, $path, $returnedPath, $isDir, $ $fileInfo = $this->getMockBuilder('OCP\Files\FileInfo') ->disableOriginalConstructor() ->getMock(); - $fileInfo->expects($this->once()) - ->method('getMimetype') - ->willReturn('audio/mp3'); $this->view->expects($this->once()) ->method('getFileInfo') @@ -650,11 +647,15 @@ public function testGetPreview($author, $fileId, $path, $returnedPath, $isDir, $ ->willReturn($fileInfo); $this->preview->expects($this->once()) - ->method('isMimeSupported') - ->with('audio/mp3') + ->method('isAvailable') + ->with($fileInfo) ->willReturn($isMimeSup); if (!$isMimeSup) { + $fileInfo->expects($this->once()) + ->method('getMimetype') + ->willReturn('audio/mp3'); + $controller->expects($this->once()) ->method('getPreviewPathFromMimeType') ->with('audio/mp3')