Merge pull request #14919 from rullzer/files_thumbnail_fix

Files thumbnail fix

apps/files/appinfo/application.php

@@ -8,7 +8,6 @@
 
 namespace OCA\Files\Appinfo;
 
-use OC\AppFramework\Utility\SimpleContainer;
 use OCA\Files\Controller\ApiController;
 use OCP\AppFramework\App;
 use \OCA\Files\Service\TagService;
@@ -18,15 +17,17 @@ class Application extends App {
 	public function __construct(array $urlParams=array()) {
 		parent::__construct('files', $urlParams);
 		$container = $this->getContainer();
+		$server = $container->getServer();
 
 		/**
 		 * Controllers
 		 */
-		$container->registerService('APIController', function (IContainer $c) {
+		$container->registerService('APIController', function (IContainer $c) use ($server) {
 			return new ApiController(
 				$c->query('AppName'),
 				$c->query('Request'),
-				$c->query('TagService')
+				$c->query('TagService'),
+				$server->getPreviewManager()
 			);
 		});
 

apps/files/controller/apicontroller.php

@@ -11,11 +11,11 @@ namespace OCA\Files\Controller;
 use OCP\AppFramework\Http;
 use OCP\AppFramework\Controller;
 use OCP\IRequest;
-use OCP\AppFramework\Http\JSONResponse;
 use OCP\AppFramework\Http\DataResponse;
+use OCP\AppFramework\Http\DataDisplayResponse;
 use OCP\AppFramework\Http\DownloadResponse;
-use OC\Preview;
 use OCA\Files\Service\TagService;
+use OCP\IPreview;
 
 /**
  * Class ApiController
@@ -25,17 +25,22 @@ use OCA\Files\Service\TagService;
 class ApiController extends Controller {
 	/** @var TagService */
 	private $tagService;
+	/** @var IPreview */
+	private $previewManager;
 
 	/**
 	 * @param string $appName
 	 * @param IRequest $request
 	 * @param TagService $tagService
+	 * @param IPreview $previewManager
 	 */
 	public function __construct($appName,
 								IRequest $request,
-								TagService $tagService){
+								TagService $tagService,
+								IPreview $previewManager){
 		parent::__construct($appName, $request);
 		$this->tagService = $tagService;
+		$this->previewManager = $previewManager;
 	}
 
 	/**
@@ -49,19 +54,18 @@ class ApiController extends Controller {
 	 * @param int $x
 	 * @param int $y
 	 * @param string $file URL-encoded filename
-	 * @return JSONResponse|DownloadResponse
+	 * @return DataResponse|DataDisplayResponse
 	 */
 	public function getThumbnail($x, $y, $file) {
 		if($x < 1 || $y < 1) {
-			return new JSONResponse('Requested size must be numeric and a positive value.', Http::STATUS_BAD_REQUEST);
+			return new DataResponse(['message' => 'Requested size must be numeric and a positive value.'], Http::STATUS_BAD_REQUEST);
 		}
 
-		try {
-			$preview = new Preview('', 'files', urldecode($file), $x, $y, true);
-			echo($preview->showPreview('image/png'));
-			return new DownloadResponse(urldecode($file).'.png', 'image/png');
-		} catch (\Exception $e) {
-			return new JSONResponse('File not found.', Http::STATUS_NOT_FOUND);
+		$preview = $this->previewManager->createPreview('files/'.urldecode($file), $x, $y, true);
+		if ($preview->valid()) {
+			return new DataDisplayResponse($preview->data(), Http::STATUS_OK, ['Content-Type' => 'image/png']);
+		} else {
+			return new DataResponse(['message' => 'File not found.'], Http::STATUS_NOT_FOUND);
 		}
 	}
 

apps/files/tests/controller/apicontrollertest.php

@@ -17,6 +17,8 @@ use Test\TestCase;
 use OCP\IRequest;
 use OCA\Files\Service\TagService;
 use OCP\AppFramework\Http\DataResponse;
+use OCP\IPreview;
+use OCP\Image;
 
 /**
  * Class ApiController
@@ -30,6 +32,8 @@ class ApiControllerTest extends TestCase {
 	private $request;
 	/** @var TagService */
 	private $tagService;
+	/** @var IPreview */
+	private $preview;
 	/** @var ApiController */
 	private $apiController;
 
@@ -40,11 +44,15 @@ class ApiControllerTest extends TestCase {
 		$this->tagService = $this->getMockBuilder('\OCA\Files\Service\TagService')
 			->disableOriginalConstructor()
 			->getMock();
+		$this->preview = $this->getMockBuilder('\OCP\IPreview')
+			->disableOriginalConstructor()
+			->getMock();
 
 		$this->apiController = new ApiController(
 			$this->appName,
 			$this->request,
-			$this->tagService
+			$this->tagService,
+			$this->preview
 		);
 	}
 
@@ -240,4 +248,29 @@ class ApiControllerTest extends TestCase {
 		$expected = new DataResponse(['message' => 'My error message'], Http::STATUS_NOT_FOUND);
 		$this->assertEquals($expected, $this->apiController->updateFileTags('/path.txt', ['Tag1', 'Tag2']));
 	}
+
+	public function testGetThumbnailInvalidSize() {
+		$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() {
+		$this->preview->expects($this->once())
+			->method('createPreview')
+			->with('files/unknown.jpg', 10, 10, true)
+			->willReturn(new Image);
+		$expected = new DataResponse(['message' => 'File not found.'], Http::STATUS_NOT_FOUND);
+		$this->assertEquals($expected, $this->apiController->getThumbnail(10, 10, 'unknown.jpg'));
+	}
+
+	public function testGetThumbnail() {
+		$this->preview->expects($this->once())
+			->method('createPreview')
+			->with('files/known.jpg', 10, 10, true)
+			->willReturn(new Image(\OC::$SERVERROOT.'/tests/data/testimage.jpg'));
+
+		$ret = $this->apiController->getThumbnail(10, 10, 'known.jpg');
+
+		$this->assertEquals(Http::STATUS_OK, $ret->getStatus());
+	}
 }