From 608456d2a90088e551de48439038da18a4c2a037 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Wed, 23 May 2018 14:25:51 +0200 Subject: [PATCH] Previews on for all trashbin files * Previews possible for all files in the trashbin * Set caching * Use the fileid to find the file * Fix test Signed-off-by: Roeland Jago Douma --- apps/files_trashbin/appinfo/routes.php | 2 +- apps/files_trashbin/js/filelist.js | 2 +- .../lib/Controller/PreviewController.php | 69 +++++++++++-------- apps/files_trashbin/lib/Helper.php | 3 +- .../Controller/PreviewControllerTest.php | 52 ++++++++------ 5 files changed, 73 insertions(+), 55 deletions(-) diff --git a/apps/files_trashbin/appinfo/routes.php b/apps/files_trashbin/appinfo/routes.php index 9243ee7e91..21b4bc2d8c 100644 --- a/apps/files_trashbin/appinfo/routes.php +++ b/apps/files_trashbin/appinfo/routes.php @@ -29,7 +29,7 @@ $application->registerRoutes($this, [ 'routes' => [ [ 'name' => 'Preview#getPreview', - 'url' => '/ajax/preview.php', + 'url' => '/preview', 'verb' => 'GET', ], ], diff --git a/apps/files_trashbin/js/filelist.js b/apps/files_trashbin/js/filelist.js index 510ab2c21b..4846c2361f 100644 --- a/apps/files_trashbin/js/filelist.js +++ b/apps/files_trashbin/js/filelist.js @@ -260,7 +260,7 @@ }, generatePreviewUrl: function(urlSpec) { - return OC.generateUrl('/apps/files_trashbin/ajax/preview.php?') + $.param(urlSpec); + return OC.generateUrl('/apps/files_trashbin/preview?') + $.param(urlSpec); }, getDownloadUrl: function() { diff --git a/apps/files_trashbin/lib/Controller/PreviewController.php b/apps/files_trashbin/lib/Controller/PreviewController.php index 8c3ff292b9..8a1b31703b 100644 --- a/apps/files_trashbin/lib/Controller/PreviewController.php +++ b/apps/files_trashbin/lib/Controller/PreviewController.php @@ -1,4 +1,5 @@ * @@ -26,6 +27,7 @@ namespace OCA\Files_Trashbin\Controller; use OCP\AppFramework\Controller; use OCP\AppFramework\Http; use OCP\AppFramework\Http\DataResponse; +use OCP\AppFramework\Utility\ITimeFactory; use OCP\Files\File; use OCP\Files\Folder; use OCP\Files\IMimeTypeDetector; @@ -48,45 +50,36 @@ class PreviewController extends Controller { /** @var IPreview */ private $previewManager; - /** - * @param string $appName - * @param IRequest $request - * @param IRootFolder $rootFolder - * @param $userId - * @param IMimeTypeDetector $mimeTypeDetector - * @param IPreview $previewManager - */ - public function __construct($appName, + /** @var ITimeFactory */ + private $time; + + public function __construct(string $appName, IRequest $request, IRootFolder $rootFolder, - $userId, + string $userId, IMimeTypeDetector $mimeTypeDetector, - IPreview $previewManager) { + IPreview $previewManager, + ITimeFactory $time) { parent::__construct($appName, $request); $this->rootFolder = $rootFolder; $this->userId = $userId; $this->mimeTypeDetector = $mimeTypeDetector; $this->previewManager = $previewManager; + $this->time = $time; } /** * @NoAdminRequired * @NoCSRFRequired * - * @param string $file - * @param int $x - * @param int $y * @return DataResponse|Http\FileDisplayResponse */ public function getPreview( - $file = '', - $x = 44, - $y = 44 + int $fileId, + int $x = 44, + int $y = 44 ) { - if ($file === '') { - return new DataResponse([], Http::STATUS_BAD_REQUEST); - } if ($x === 0 || $y === 0) { return new DataResponse([], Http::STATUS_BAD_REQUEST); @@ -96,23 +89,41 @@ class PreviewController extends Controller { $userFolder = $this->rootFolder->getUserFolder($this->userId); /** @var Folder $trash */ $trash = $userFolder->getParent()->get('files_trashbin/files'); - $trashFile = $trash->get($file); + $trashFiles = $trash->getById($fileId); + + if (empty($trashFiles)) { + throw new NotFoundException(); + } + + $trashFile = array_pop($trashFiles); if ($trashFile instanceof Folder) { return new DataResponse([], Http::STATUS_BAD_REQUEST); } - /** @var File $trashFile */ - $fileName = $trashFile->getName(); - $i = strrpos($fileName, '.'); - if ($i !== false) { - $fileName = substr($fileName, 0, $i); + /* + * Files in the root of the trashbin are timetamped. + * So we have to strip that in order to properly detect the mimetype of the file. + */ + if ($trashFile->getParent()->getPath() === $trash->getPath()) { + /** @var File $trashFile */ + $fileName = $trashFile->getName(); + $i = strrpos($fileName, '.'); + if ($i !== false) { + $fileName = substr($fileName, 0, $i); + } + + $mimeType = $this->mimeTypeDetector->detectPath($fileName); + } else { + $mimeType = $this->mimeTypeDetector->detectPath($trashFile->getName()); } - $mimeType = $this->mimeTypeDetector->detectPath($fileName); - $f = $this->previewManager->getPreview($trashFile, $x, $y, true, IPreview::MODE_FILL, $mimeType); - return new Http\FileDisplayResponse($f, Http::STATUS_OK, ['Content-Type' => $f->getMimeType()]); + $response = new Http\FileDisplayResponse($f, Http::STATUS_OK, ['Content-Type' => $f->getMimeType()]); + + // Cache previews for 24H + $response->cacheFor(3600 * 24); + return $response; } catch (NotFoundException $e) { return new DataResponse([], Http::STATUS_NOT_FOUND); } catch (\InvalidArgumentException $e) { diff --git a/apps/files_trashbin/lib/Helper.php b/apps/files_trashbin/lib/Helper.php index 01a4fd231e..071a1a7766 100644 --- a/apps/files_trashbin/lib/Helper.php +++ b/apps/files_trashbin/lib/Helper.php @@ -116,10 +116,9 @@ class Helper { */ public static function formatFileInfos($fileInfos) { $files = array(); - $id = 0; foreach ($fileInfos as $i) { $entry = \OCA\Files\Helper::formatFileInfo($i); - $entry['id'] = $id++; + $entry['id'] = $i->getId(); $entry['etag'] = $entry['mtime']; // add fake etag, it is only needed to identify the preview image $entry['permissions'] = \OCP\Constants::PERMISSION_READ; $files[] = $entry; diff --git a/apps/files_trashbin/tests/Controller/PreviewControllerTest.php b/apps/files_trashbin/tests/Controller/PreviewControllerTest.php index 685e4cebfd..9ceef07e7d 100644 --- a/apps/files_trashbin/tests/Controller/PreviewControllerTest.php +++ b/apps/files_trashbin/tests/Controller/PreviewControllerTest.php @@ -26,6 +26,7 @@ use OCA\Files_Trashbin\Controller\PreviewController; use OCP\AppFramework\Http; use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\Http\FileDisplayResponse; +use OCP\AppFramework\Utility\ITimeFactory; use OCP\Files\File; use OCP\Files\Folder; use OCP\Files\IMimeTypeDetector; @@ -50,6 +51,9 @@ class PreviewControllerTest extends TestCase { /** @var IPreview|\PHPUnit_Framework_MockObject_MockObject */ private $previewManager; + /** @var ITimeFactory|\PHPUnit_Framework_MockObject_MockObject */ + private $time; + /** @var PreviewController */ private $controller; @@ -60,6 +64,7 @@ class PreviewControllerTest extends TestCase { $this->userId = 'user'; $this->mimeTypeDetector = $this->createMock(IMimeTypeDetector::class); $this->previewManager = $this->createMock(IPreview::class); + $this->time = $this->createMock(ITimeFactory::class); $this->controller = new PreviewController( 'files_versions', @@ -67,26 +72,20 @@ class PreviewControllerTest extends TestCase { $this->rootFolder, $this->userId, $this->mimeTypeDetector, - $this->previewManager + $this->previewManager, + $this->time ); } - public function testInvalidFile() { - $res = $this->controller->getPreview(''); - $expected = new DataResponse([], Http::STATUS_BAD_REQUEST); - - $this->assertEquals($expected, $res); - } - public function testInvalidWidth() { - $res = $this->controller->getPreview('file', 0); + $res = $this->controller->getPreview(42, 0); $expected = new DataResponse([], Http::STATUS_BAD_REQUEST); $this->assertEquals($expected, $res); } public function testInvalidHeight() { - $res = $this->controller->getPreview('file', 10, 0); + $res = $this->controller->getPreview(42, 10, 0); $expected = new DataResponse([], Http::STATUS_BAD_REQUEST); $this->assertEquals($expected, $res); @@ -111,12 +110,15 @@ class PreviewControllerTest extends TestCase { ->willReturn('myMime'); $file = $this->createMock(File::class); - $trash->method('get') - ->with($this->equalTo('file.1234')) - ->willReturn($file); + $trash->method('getById') + ->with($this->equalTo(42)) + ->willReturn([$file]); $file->method('getName') ->willReturn('file.1234'); + $file->method('getParent') + ->willReturn($trash); + $preview = $this->createMock(ISimpleFile::class); $this->previewManager->method('getPreview') ->with($this->equalTo($file), 10, 10, true, IPreview::MODE_FILL, 'myMime') @@ -124,8 +126,14 @@ class PreviewControllerTest extends TestCase { $preview->method('getMimeType') ->willReturn('previewMime'); - $res = $this->controller->getPreview('file.1234', 10, 10); + $this->time->method('getTime') + ->willReturn(1337); + + $this->overwriteService(ITimeFactory::class, $this->time); + + $res = $this->controller->getPreview(42, 10, 10); $expected = new FileDisplayResponse($preview, Http::STATUS_OK, ['Content-Type' => 'previewMime']); + $expected->cacheFor(3600 * 24); $this->assertEquals($expected, $res); } @@ -144,11 +152,11 @@ class PreviewControllerTest extends TestCase { ->with('files_trashbin/files') ->willReturn($trash); - $trash->method('get') - ->with($this->equalTo('file.1234')) - ->willThrowException(new NotFoundException()); + $trash->method('getById') + ->with($this->equalTo(42)) + ->willReturn([]); - $res = $this->controller->getPreview('file.1234', 10, 10); + $res = $this->controller->getPreview(42, 10, 10); $expected = new DataResponse([], Http::STATUS_NOT_FOUND); $this->assertEquals($expected, $res); @@ -169,11 +177,11 @@ class PreviewControllerTest extends TestCase { ->willReturn($trash); $folder = $this->createMock(Folder::class); - $trash->method('get') - ->with($this->equalTo('folder')) - ->willReturn($folder); + $trash->method('getById') + ->with($this->equalTo(43)) + ->willReturn([$folder]); - $res = $this->controller->getPreview('folder', 10, 10); + $res = $this->controller->getPreview(43, 10, 10); $expected = new DataResponse([], Http::STATUS_BAD_REQUEST); $this->assertEquals($expected, $res);