Merge pull request #1121 from nextcloud/avatar_304

Proper 304 response

5 files changed, 184 insertions, 8 deletions

diff --git a/core/Controller/AvatarController.php b/core/Controller/AvatarController.php
index 484b6f524bf..b27d1182225 100644
--- a/core/Controller/AvatarController.php
+++ b/core/Controller/AvatarController.php
@@ -31,6 +31,7 @@ use OC\AppFramework\Utility\TimeFactory;
 use OCP\AppFramework\Controller;
 use OCP\AppFramework\Http;
 use OCP\AppFramework\Http\DataDisplayResponse;
+use OCP\AppFramework\Http\FileDisplayResponse;
 use OCP\AppFramework\Http\JSONResponse;
 use OCP\Files\File;
 use OCP\Files\IRootFolder;
@@ -118,7 +119,7 @@ class AvatarController extends Controller {
 	 *
 	 * @param string $userId
 	 * @param int $size
-	 * @return JSONResponse|DataDisplayResponse
+	 * @return JSONResponse|FileDisplayResponse
 	 */
 	public function getAvatar($userId, $size) {
 		if ($size > 2048) {
@@ -129,25 +130,19 @@ class AvatarController extends Controller {
 
 		try {
 			$avatar = $this->avatarManager->getAvatar($userId)->getFile($size);
-			$resp = new DataDisplayResponse($avatar->getContent(),
+			$resp = new FileDisplayResponse($avatar,
 				Http::STATUS_OK,
 				['Content-Type' => $avatar->getMimeType()]);
-			$resp->setETag($avatar->getEtag());
 
 			// Let cache this!
 			$resp->addHeader('Pragma', 'public');
 			// Cache for 15 minutes
 			$resp->cacheFor(900);
-			// Set last modified
-			$lastModified = new \DateTime();
-			$lastModified->setTimestamp($avatar->getMTime());
-			$resp->setLastModified($lastModified);
 
 			$expires = new \DateTime();
 			$expires->setTimestamp($this->timeFactory->getTime());
 			$expires->add(new \DateInterval('PT15M'));
 			$resp->addHeader('Expires', $expires->format(\DateTime::RFC2822));
-
 		} catch (NotFoundException $e) {
 			$user = $this->userManager->get($userId);
 			$resp = new JSONResponse([
diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php
index 2700c87ec00..8f05b6b0c9c 100644
--- a/lib/composer/composer/autoload_classmap.php
+++ b/lib/composer/composer/autoload_classmap.php
@@ -26,6 +26,7 @@ return array(
     'OCP\\AppFramework\\Http\\DataResponse' => $baseDir . '/lib/public/AppFramework/Http/DataResponse.php',
     'OCP\\AppFramework\\Http\\DownloadResponse' => $baseDir . '/lib/public/AppFramework/Http/DownloadResponse.php',
     'OCP\\AppFramework\\Http\\EmptyContentSecurityPolicy' => $baseDir . '/lib/public/AppFramework/Http/EmptyContentSecurityPolicy.php',
+    'OCP\\AppFramework\\Http\\FileDisplayResponse' => $baseDir . '/lib/public/AppFramework/Http/FileDisplayResponse.php',
     'OCP\\AppFramework\\Http\\ICallbackResponse' => $baseDir . '/lib/public/AppFramework/Http/ICallbackResponse.php',
     'OCP\\AppFramework\\Http\\IOutput' => $baseDir . '/lib/public/AppFramework/Http/IOutput.php',
     'OCP\\AppFramework\\Http\\JSONResponse' => $baseDir . '/lib/public/AppFramework/Http/JSONResponse.php',
diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php
index 3797711f914..ea02c14fe25 100644
--- a/lib/composer/composer/autoload_static.php
+++ b/lib/composer/composer/autoload_static.php
@@ -56,6 +56,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c
         'OCP\\AppFramework\\Http\\DataResponse' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http/DataResponse.php',
         'OCP\\AppFramework\\Http\\DownloadResponse' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http/DownloadResponse.php',
         'OCP\\AppFramework\\Http\\EmptyContentSecurityPolicy' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http/EmptyContentSecurityPolicy.php',
+        'OCP\\AppFramework\\Http\\FileDisplayResponse' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http/FileDisplayResponse.php',
         'OCP\\AppFramework\\Http\\ICallbackResponse' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http/ICallbackResponse.php',
         'OCP\\AppFramework\\Http\\IOutput' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http/IOutput.php',
         'OCP\\AppFramework\\Http\\JSONResponse' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http/JSONResponse.php',
diff --git a/lib/public/AppFramework/Http/FileDisplayResponse.php b/lib/public/AppFramework/Http/FileDisplayResponse.php
new file mode 100644
index 00000000000..22171e2b379
--- /dev/null
+++ b/lib/public/AppFramework/Http/FileDisplayResponse.php
@@ -0,0 +1,70 @@
+<?php
+/**
+ * @copyright 2016 Roeland Jago Douma <roeland@famdouma.nl>
+ *
+ * @author Roeland Jago Douma <roeland@famdouma.nl>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+namespace OCP\AppFramework\Http;
+
+use OCP\AppFramework\Http;
+use OCP\Files\File;
+
+/**
+ * Class FileDisplayResponse
+ *
+ * @package OCP\AppFramework\Http
+ * @since 9.2.0
+ */
+class FileDisplayResponse extends Response implements ICallbackResponse {
+
+	/** @var File */
+	private $file;
+
+	/**
+	 * FileDisplayResponse constructor.
+	 *
+	 * @param File $file
+	 * @param int $statusCode
+	 * @param array $headers
+	 * @since 9.2.0
+	 */
+	public function __construct(File $file, $statusCode=Http::STATUS_OK,
+								$headers=[]) {
+		$this->file = $file;
+		$this->setStatus($statusCode);
+		$this->setHeaders(array_merge($this->getHeaders(), $headers));
+		$this->addHeader('Content-Disposition', 'inline; filename="' . rawurldecode($file->getName()) . '"');
+
+		$this->setETag($file->getEtag());
+		$lastModified = new \DateTime();
+		$lastModified->setTimestamp($file->getMTime());
+		$this->setLastModified($lastModified);
+	}
+
+	/**
+	 * @param IOutput $output
+	 * @since 9.2.0
+	 */
+	public function callback(IOutput $output) {
+		if ($output->getHttpResponseCode() !== Http::STATUS_NOT_MODIFIED) {
+			$output->setHeader('Content-Length: ' . $this->file->getSize());
+			$output->setOutput($this->file->getContent());
+		}
+	}
+}
diff --git a/tests/lib/AppFramework/Http/FileDisplayResponseTest.php b/tests/lib/AppFramework/Http/FileDisplayResponseTest.php
new file mode 100644
index 00000000000..49334c17e74
--- /dev/null
+++ b/tests/lib/AppFramework/Http/FileDisplayResponseTest.php
@@ -0,0 +1,109 @@
+<?php
+/**
+ * @copyright 2016 Roeland Jago Douma <roeland@famdouma.nl>
+ *
+ * @author Roeland Jago Douma <roeland@famdouma.nl>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+namespace Test\AppFramework\Http;
+
+use OCP\AppFramework\Http;
+use OCP\AppFramework\Http\FileDisplayResponse;
+use OCP\Files\File;
+
+class FileDisplayResponseTest extends \Test\TestCase {
+	/** @var File|\PHPUnit_Framework_MockObject_MockObject */
+	private $file;
+
+	/** @var FileDisplayResponse */
+	private $response;
+
+	public function setup() {
+		$this->file = $this->getMockBuilder('OCP\Files\File')
+			->getMock();
+
+		$this->file->expects($this->once())
+			->method('getETag')
+			->willReturn('myETag');
+		$this->file->expects($this->once())
+			->method('getName')
+			->willReturn('myFileName');
+		$this->file->expects($this->once())
+			->method('getMTime')
+			->willReturn(1464825600);
+
+		$this->response = new FileDisplayResponse($this->file);
+	}
+
+	public function testHeader() {
+		$headers = $this->response->getHeaders();
+		$this->assertArrayHasKey('Content-Disposition', $headers);
+		$this->assertSame('inline; filename="myFileName"', $headers['Content-Disposition']);
+	}
+
+	public function testETag() {
+		$this->assertSame('myETag', $this->response->getETag());
+	}
+
+	public function testLastModified() {
+		$lastModified = $this->response->getLastModified();
+		$this->assertNotNull($lastModified);
+		$this->assertSame(1464825600, $lastModified->getTimestamp());
+	}
+
+	public function test304() {
+		$output = $this->getMockBuilder('OCP\AppFramework\Http\IOutput')
+			->disableOriginalConstructor()
+			->getMock();
+
+		$output->expects($this->any())
+			->method('getHttpResponseCode')
+			->willReturn(Http::STATUS_NOT_MODIFIED);
+		$output->expects($this->never())
+			->method('setOutput');
+		$this->file->expects($this->never())
+			->method('getContent');
+
+		$this->response->callback($output);
+	}
+
+
+	public function testNon304() {
+		$output = $this->getMockBuilder('OCP\AppFramework\Http\IOutput')
+			->disableOriginalConstructor()
+			->getMock();
+
+		$output->expects($this->any())
+			->method('getHttpResponseCode')
+			->willReturn(Http::STATUS_OK);
+		$output->expects($this->once())
+			->method('setOutput')
+			->with($this->equalTo('my data'));
+		$output->expects($this->once())
+			->method('setHeader')
+			->with($this->equalTo('Content-Length: 42'));
+		$this->file->expects($this->once())
+			->method('getContent')
+			->willReturn('my data');
+		$this->file->expects($this->any())
+			->method('getSize')
+			->willReturn(42);
+
+		$this->response->callback($output);
+	}
+}