From 45f7a8bf954938a1ded52df4ba94cfaeddf36b59 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Thu, 11 Oct 2018 15:41:20 +0200 Subject: [PATCH 1/3] Add the mimetype to the preview arrays Signed-off-by: Joas Schilling --- docs/endpoint-v2.md | 1 + lib/Controller/APIv2.php | 74 ++++++++++++++-------------------------- 2 files changed, 26 insertions(+), 49 deletions(-) diff --git a/docs/endpoint-v2.md b/docs/endpoint-v2.md index d785328a0..0ccbe24ac 100644 --- a/docs/endpoint-v2.md +++ b/docs/endpoint-v2.md @@ -98,6 +98,7 @@ Field name | Type | Value description ---------- | ---- | ----------------- `source` | string | Full URL of the image to be displayed `link` | string | Full URL the preview should be wrapped in +`mimeType` | string | The mime type of the file (not the preview) `isMimeTypeIcon` | bool | True if `source` points to a mime type icon instead of a real preview In case the endpoint returns more fields, they should be ignored and are deprecated (only for backwards compatibility usage) or internal. diff --git a/lib/Controller/APIv2.php b/lib/Controller/APIv2.php index a923f85ff..4f3dc2b48 100644 --- a/lib/Controller/APIv2.php +++ b/lib/Controller/APIv2.php @@ -1,4 +1,5 @@ filter = is_string($filter) ? $filter : 'all'; + $this->filter = \is_string($filter) ? $filter : 'all'; if ($this->filter !== $this->data->validateFilter($this->filter)) { - throw new InvalidFilterException(); + throw new InvalidFilterException('Invalid filter'); } $this->since = (int) $since; $this->limit = (int) $limit; $this->loadPreviews = (bool) $previews; $this->objectType = (string) $objectType; $this->objectId = (int) $objectId; - $this->sort = in_array($sort, ['asc', 'desc'], true) ? $sort : 'desc'; + $this->sort = \in_array($sort, ['asc', 'desc'], true) ? $sort : 'desc'; if (($this->objectType !== '' && $this->objectId === 0) || ($this->objectType === '' && $this->objectId !== 0)) { // Only allowed together @@ -173,7 +174,7 @@ protected function validateParameters($filter, $since, $limit, $previews, $objec $this->user = $user->getUID(); } else { // No user logged in - throw new \OutOfBoundsException(); + throw new \OutOfBoundsException('Not logged in'); } } @@ -188,7 +189,7 @@ protected function validateParameters($filter, $since, $limit, $previews, $objec * @param string $sort * @return DataResponse */ - public function getDefault($since = 0, $limit = 50, $previews = false, $object_type = '', $object_id = 0, $sort = 'desc') { + public function getDefault($since = 0, $limit = 50, $previews = false, $object_type = '', $object_id = 0, $sort = 'desc'): DataResponse { return $this->get('all', $since, $limit, $previews, $object_type, $object_id, $sort); } @@ -204,7 +205,7 @@ public function getDefault($since = 0, $limit = 50, $previews = false, $object_t * @param string $sort * @return DataResponse */ - public function getFilter($filter, $since = 0, $limit = 50, $previews = false, $object_type = '', $object_id = 0, $sort = 'desc') { + public function getFilter($filter, $since = 0, $limit = 50, $previews = false, $object_type = '', $object_id = 0, $sort = 'desc'): DataResponse { return $this->get($filter, $since, $limit, $previews, $object_type, $object_id, $sort); } @@ -213,7 +214,7 @@ public function getFilter($filter, $since = 0, $limit = 50, $previews = false, $ * * @return DataResponse */ - public function listFilters() { + public function listFilters(): DataResponse { $filters = $this->activityManager->getFilters(); $filters = array_map(function(IFilter $filter) { @@ -247,7 +248,7 @@ public function listFilters() { * @param string $sort * @return DataResponse */ - protected function get($filter, $since, $limit, $previews, $filterObjectType, $filterObjectId, $sort) { + protected function get($filter, $since, $limit, $previews, $filterObjectType, $filterObjectId, $sort): DataResponse { try { $this->validateParameters($filter, $since, $limit, $previews, $filterObjectType, $filterObjectId, $sort); } catch (InvalidFilterException $e) { @@ -293,7 +294,7 @@ protected function get($filter, $since, $limit, $previews, $filterObjectType, $f if ($this->loadPreviews) { $activity['previews'] = []; if ($activity['object_type'] === 'files') { - if (!empty($activity['objects']) && is_array($activity['objects'])) { + if (!empty($activity['objects']) && \is_array($activity['objects'])) { foreach ($activity['objects'] as $objectId => $objectName) { if (((int) $objectId) === 0 || $objectName === '') { // No file, no preview @@ -315,13 +316,7 @@ protected function get($filter, $since, $limit, $previews, $filterObjectType, $f return new DataResponse($preparedActivities, Http::STATUS_OK, $headers); } - /** - * @param array $headers - * @param bool $hasMoreActivities - * @param array $data - * @return array - */ - protected function generateHeaders(array $headers, $hasMoreActivities, array $data) { + protected function generateHeaders(array $headers, bool $hasMoreActivities, array $data): array { if ($hasMoreActivities && isset($headers['X-Activity-Last-Given'])) { // Set the "Link" header for the next page $nextPageParameters = [ @@ -354,13 +349,7 @@ protected function generateHeaders(array $headers, $hasMoreActivities, array $da return $headers; } - /** - * @param string $owner - * @param int $fileId - * @param string $filePath - * @return array - */ - protected function getPreview($owner, $fileId, $filePath) { + protected function getPreview(string $owner, int $fileId, string $filePath): array { $info = $this->infoCache->getInfoById($owner, $fileId, $filePath); if (!$info['exists'] || $info['view'] !== '') { @@ -370,55 +359,48 @@ protected function getPreview($owner, $fileId, $filePath) { $preview = [ 'link' => $this->getPreviewLink($info['path'], $info['is_dir'], $info['view']), 'source' => '', + 'mimeType' => 'application/octet-stream', 'isMimeTypeIcon' => true, ]; // show a preview image if the file still exists if ($info['is_dir']) { $preview['source'] = $this->getPreviewPathFromMimeType('dir'); + $preview['mimeType'] = 'dir'; } else { $this->view->chroot('/' . $owner . '/files'); $fileInfo = $this->view->getFileInfo($info['path']); - if (!($fileInfo instanceof FileInfo)) { - $pathPreview = $this->getPreviewFromPath($filePath, $info); - $preview['source'] = $pathPreview['source']; + if (!$fileInfo instanceof FileInfo) { + $preview = $this->getPreviewFromPath($filePath, $info); } else if ($this->preview->isAvailable($fileInfo)) { - $preview['isMimeTypeIcon'] = false; $preview['source'] = $this->urlGenerator->linkToRouteAbsolute('core.Preview.getPreview', [ 'file' => $info['path'], 'c' => $this->view->getETag($info['path']), 'x' => 150, 'y' => 150, ]); + $preview['mimeType'] = $fileInfo->getMimetype(); + $preview['isMimeTypeIcon'] = false; } else { $preview['source'] = $this->getPreviewPathFromMimeType($fileInfo->getMimetype()); + $preview['mimeType'] = $fileInfo->getMimetype(); } } return $preview; } - /** - * @param string $filePath - * @param array $info - * @return array - */ - protected function getPreviewFromPath($filePath, $info) { + protected function getPreviewFromPath(string $filePath, array $info): array { $mimeType = $info['is_dir'] ? 'dir' : $this->mimeTypeDetector->detectPath($filePath); - $preview = [ + return [ 'link' => $this->getPreviewLink($info['path'], $info['is_dir'], $info['view']), 'source' => $this->getPreviewPathFromMimeType($mimeType), + 'mimeType' => $mimeType, 'isMimeTypeIcon' => true, ]; - - return $preview; } - /** - * @param string $mimeType - * @return string - */ - protected function getPreviewPathFromMimeType($mimeType) { + protected function getPreviewPathFromMimeType(string $mimeType): string { $mimeTypeIcon = $this->mimeTypeDetector->mimeTypeIcon($mimeType); if (substr($mimeTypeIcon, -4) === '.png') { $mimeTypeIcon = substr($mimeTypeIcon, 0, -4) . '.svg'; @@ -427,18 +409,12 @@ protected function getPreviewPathFromMimeType($mimeType) { return $this->urlGenerator->getAbsoluteURL($mimeTypeIcon); } - /** - * @param string $path - * @param bool $isDir - * @param string $view - * @return string - */ - protected function getPreviewLink($path, $isDir, $view) { + protected function getPreviewLink(string $path, bool $isDir, string $view): string { $params = [ 'dir' => $path, ]; if (!$isDir) { - $params['dir'] = (substr_count($path, '/') === 1) ? '/' : dirname($path); + $params['dir'] = (substr_count($path, '/') === 1) ? '/' : \dirname($path); $params['scrollto'] = basename($path); } if ($view !== '') { From 09446c23ef05204e7372872979f85e29c4f6d9c1 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Thu, 11 Oct 2018 16:05:03 +0200 Subject: [PATCH 2/3] Add the view and the fileId too Signed-off-by: Joas Schilling --- docs/endpoint-v2.md | 19 ++++++++++++------- lib/Controller/APIv2.php | 10 +++++++--- 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/docs/endpoint-v2.md b/docs/endpoint-v2.md index 0ccbe24ac..2c87650cc 100644 --- a/docs/endpoint-v2.md +++ b/docs/endpoint-v2.md @@ -99,6 +99,8 @@ Field name | Type | Value description `source` | string | Full URL of the image to be displayed `link` | string | Full URL the preview should be wrapped in `mimeType` | string | The mime type of the file (not the preview) +`fileId` | int | The if of the actual file +`view` | string | The view where the file can be found (either `files` or `trashbin`) `isMimeTypeIcon` | bool | True if `source` points to a mime type icon instead of a real preview In case the endpoint returns more fields, they should be ignored and are deprecated (only for backwards compatibility usage) or internal. @@ -113,15 +115,15 @@ In case the endpoint returns more fields, they should be ignored and are depreca "type": "file_created", "user": "test1", "affecteduser": "admin", - "subject": "test1 created hello.jpg", + "subject": "test1 created hello.txt", "subject_rich": { "0": "test1 created {file1}", "1": { "file1": { "type": "file", "id": 23, - "name": "hello.jpg", - "path": "\/test\/hello.jpg" + "name": "hello.txt", + "path": "\/test\/hello.txt" } } }, @@ -134,12 +136,15 @@ In case the endpoint returns more fields, they should be ignored and are depreca "link": "", "object_type": "files", "object_id": 23, - "object_name": "\/test\/hello.jpg", + "object_name": "\/test\/hello.txt", "previews": [ { - "link": "https:\/\/localhost\/index.php\/apps\/files\/?dir=\/test&scrollto=hello.jpg", - "source": "https:\/\/localhost\/index.php\/core\/preview.png?file=\/hello.jpg&x=150&y=150", - "isMimeTypeIcon": false + "link": "https:\/\/localhost\/index.php\/apps\/files\/?dir=\/test&scrollto=hello.txt", + "source": "https:\/\/localhost\/index.php\/core\/preview.png?file=\/hello.txt&x=150&y=150", + "mimeType": "text/plain", + "view": "files", + "fileId": 23, + "isMimeTypeIcon": false } ] } diff --git a/lib/Controller/APIv2.php b/lib/Controller/APIv2.php index 4f3dc2b48..88b3c9f86 100644 --- a/lib/Controller/APIv2.php +++ b/lib/Controller/APIv2.php @@ -353,7 +353,7 @@ protected function getPreview(string $owner, int $fileId, string $filePath): arr $info = $this->infoCache->getInfoById($owner, $fileId, $filePath); if (!$info['exists'] || $info['view'] !== '') { - return $this->getPreviewFromPath($filePath, $info); + return $this->getPreviewFromPath($fileId, $filePath, $info); } $preview = [ @@ -361,6 +361,8 @@ protected function getPreview(string $owner, int $fileId, string $filePath): arr 'source' => '', 'mimeType' => 'application/octet-stream', 'isMimeTypeIcon' => true, + 'fileId' => $fileId, + 'view' => $info['view'] ?: 'files', ]; // show a preview image if the file still exists @@ -371,7 +373,7 @@ protected function getPreview(string $owner, int $fileId, string $filePath): arr $this->view->chroot('/' . $owner . '/files'); $fileInfo = $this->view->getFileInfo($info['path']); if (!$fileInfo instanceof FileInfo) { - $preview = $this->getPreviewFromPath($filePath, $info); + $preview = $this->getPreviewFromPath($fileId, $filePath, $info); } else if ($this->preview->isAvailable($fileInfo)) { $preview['source'] = $this->urlGenerator->linkToRouteAbsolute('core.Preview.getPreview', [ 'file' => $info['path'], @@ -390,13 +392,15 @@ protected function getPreview(string $owner, int $fileId, string $filePath): arr return $preview; } - protected function getPreviewFromPath(string $filePath, array $info): array { + protected function getPreviewFromPath(int $fileId, string $filePath, array $info): array { $mimeType = $info['is_dir'] ? 'dir' : $this->mimeTypeDetector->detectPath($filePath); return [ 'link' => $this->getPreviewLink($info['path'], $info['is_dir'], $info['view']), 'source' => $this->getPreviewPathFromMimeType($mimeType), 'mimeType' => $mimeType, 'isMimeTypeIcon' => true, + 'fileId' => $fileId, + 'view' => $info['view'] ?: 'files', ]; } From 190f6c85917db60136d8fc8b70b019acfa189404 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 7 Nov 2018 12:23:53 +0100 Subject: [PATCH 3/3] Adjust unit tests Signed-off-by: Joas Schilling --- tests/Controller/APIv2Test.php | 47 ++++++++++++++++++++++++---------- 1 file changed, 33 insertions(+), 14 deletions(-) diff --git a/tests/Controller/APIv2Test.php b/tests/Controller/APIv2Test.php index 869779476..3cea5b777 100644 --- a/tests/Controller/APIv2Test.php +++ b/tests/Controller/APIv2Test.php @@ -651,7 +651,7 @@ public function testGetPreviewInvalidPaths($author, $fileId, $path, $returnedPat ]); $controller->expects($this->any()) ->method('getPreviewFromPath') - ->with($path) + ->with($fileId, $path) ->willReturn(['getPreviewFromPath']); $this->assertSame(['getPreviewFromPath'], self::invokePrivate($controller, 'getPreview', [$author, $fileId, $path])); @@ -659,10 +659,10 @@ public function testGetPreviewInvalidPaths($author, $fileId, $path, $returnedPat public function dataGetPreview() { return [ - ['author', 42, '/path', '/currentPath', true, true, false, '/preview/dir', true], - ['author', 42, '/file.txt', '/currentFile.txt', false, true, false, '/preview/mpeg', true], - ['author', 42, '/file.txt', '/currentFile.txt', false, true, true, '/preview/currentFile.txt', false], - ['author', 42, '/file.txt', '/currentFile.txt', false, false, true, 'source::getPreviewFromPath', true], + ['author', 42, '/path', '/currentPath', true, true, false, '/preview/dir', true, 'dir'], + ['author', 42, '/file.txt', '/currentFile.txt', false, true, false, '/preview/mpeg', true, 'audio/mp3'], + ['author', 42, '/file.txt', '/currentFile.txt', false, true, true, '/preview/currentFile.txt', false, 'text/plain'], + ['author', 42, '/file.txt', '/currentFile.txt', false, false, true, 'source::getPreviewFromPath', true, 'text/plain'], ]; } @@ -678,8 +678,9 @@ public function dataGetPreview() { * @param bool $isMimeSup * @param string $source * @param bool $isMimeTypeIcon + * @param string $mimeType */ - public function testGetPreview($author, $fileId, $path, $returnedPath, $isDir, $validFileInfo, $isMimeSup, $source, $isMimeTypeIcon) { + public function testGetPreview($author, $fileId, $path, $returnedPath, $isDir, $validFileInfo, $isMimeSup, $source, $isMimeTypeIcon, $mimeType) { $controller = $this->getController([ 'getPreviewLink', @@ -726,7 +727,7 @@ public function testGetPreview($author, $fileId, $path, $returnedPath, $isDir, $ ->willReturn($isMimeSup); if (!$isMimeSup) { - $fileInfo->expects($this->once()) + $fileInfo->expects($this->atLeastOnce()) ->method('getMimetype') ->willReturn('audio/mp3'); @@ -735,6 +736,10 @@ public function testGetPreview($author, $fileId, $path, $returnedPath, $isDir, $ ->with('audio/mp3') ->willReturn('/preview/mpeg'); } else { + $fileInfo->expects($this->atLeastOnce()) + ->method('getMimetype') + ->willReturn('text/plain'); + $this->urlGenerator->expects($this->once()) ->method('linkToRouteAbsolute') ->with('core.Preview.getPreview', $this->anything()) @@ -753,33 +758,44 @@ public function testGetPreview($author, $fileId, $path, $returnedPath, $isDir, $ $controller->expects($this->once()) ->method('getPreviewFromPath') - ->with($path, $this->anything()) - ->willReturn(['source' => 'source::getPreviewFromPath']); + ->with($fileId, $path, $this->anything()) + ->willReturn([ + 'link' => '/preview' . $returnedPath, + 'source' => 'source::getPreviewFromPath', + 'mimeType' => $mimeType, + 'isMimeTypeIcon' => $isMimeTypeIcon, + 'fileId' => $fileId, + 'view' => 'files', + ]); } $this->assertSame([ 'link' => '/preview' . $returnedPath, 'source' => $source, + 'mimeType' => $mimeType, 'isMimeTypeIcon' => $isMimeTypeIcon, + 'fileId' => $fileId, + 'view' => 'files', ], self::invokePrivate($controller, 'getPreview', [$author, $fileId, $path])); } public function dataGetPreviewFromPath() { return [ - ['dir', 'dir', true, ''], - ['test.txt', 'text/plain', false, 'trashbin'], - ['test.mp3', 'audio/mpeg', false, ''], + [23, 'dir', 'dir', true, ''], + [42, 'test.txt', 'text/plain', false, 'trashbin'], + [128, 'test.mp3', 'audio/mpeg', false, ''], ]; } /** * @dataProvider dataGetPreviewFromPath + * @param int $fileId * @param string $filePath * @param string $mimeType * @param bool $isDir * @param string $view */ - public function testGetPreviewFromPath($filePath, $mimeType, $isDir, $view) { + public function testGetPreviewFromPath($fileId, $filePath, $mimeType, $isDir, $view) { $controller = $this->getController([ 'getPreviewPathFromMimeType', 'getPreviewLink', @@ -801,9 +817,12 @@ public function testGetPreviewFromPath($filePath, $mimeType, $isDir, $view) { [ 'link' => 'target-link', 'source' => 'mime-type-icon', + 'mimeType' => $mimeType, 'isMimeTypeIcon' => true, + 'fileId' => $fileId, + 'view' => $view ?: 'files', ], - self::invokePrivate($controller, 'getPreviewFromPath', [$filePath, ['path' => $filePath, 'is_dir' => $isDir, 'view' => $view]]) + self::invokePrivate($controller, 'getPreviewFromPath', [$fileId, $filePath, ['path' => $filePath, 'is_dir' => $isDir, 'view' => $view]]) ); }