Merge pull request #13616 from owncloud/streamresponse

AppFramework StreamResponse

lib/private/appframework/app.php

@@ -24,9 +24,10 @@
 
 namespace OC\AppFramework;
 
-use \OC_App;
+use OC_App;
-use \OC\AppFramework\DependencyInjection\DIContainer;
+use OC\AppFramework\DependencyInjection\DIContainer;
-use \OCP\AppFramework\QueryException;
+use OCP\AppFramework\QueryException;
+use OCP\AppFramework\Http\ICallbackResponse;
 
 /**
  * Entry point for every request in your app. You can consider this as your
@@ -93,15 +94,22 @@ class App {
 		// initialize the dispatcher and run all the middleware before the controller
 		$dispatcher = $container['Dispatcher'];
 
-		list($httpHeaders, $responseHeaders, $responseCookies, $output) =
+		list(
-			$dispatcher->dispatch($controller, $methodName);
+			$httpHeaders,
+			$responseHeaders,
+			$responseCookies,
+			$output,
+			$response
+		) = $dispatcher->dispatch($controller, $methodName);
+
+		$io = $container['OCP\\AppFramework\\Http\\IOutput'];
 
 		if(!is_null($httpHeaders)) {
-			header($httpHeaders);
+			$io->setHeader($httpHeaders);
 		}
 
 		foreach($responseHeaders as $name => $value) {
-			header($name . ': ' . $value);
+			$io->setHeader($name . ': ' . $value);
 		}
 
 		foreach($responseCookies as $name => $value) {
@@ -109,12 +117,22 @@ class App {
 			if($value['expireDate'] instanceof \DateTime) {
 				$expireDate = $value['expireDate']->getTimestamp();
 			}
-			setcookie($name, $value['value'], $expireDate, $container->getServer()->getWebRoot(), null, $container->getServer()->getConfig()->getSystemValue('forcessl', false), true);
+			$io->setCookie(
+				$name,
+				$value['value'],
+				$expireDate,
+				$container->getServer()->getWebRoot(),
+				null,
+				$container->getServer()->getConfig()->getSystemValue('forcessl', false),
+				true
+			);
 		}
 
-		if(!is_null($output)) {
+		if ($response instanceof ICallbackResponse) {
-			header('Content-Length: ' . strlen($output));
+			$response->callback($io);
-			print($output);
+		} else if(!is_null($output)) {
+			$io->setHeader('Content-Length: ' . strlen($output));
+			$io->setOutput($output);
 		}
 
 	}

lib/private/appframework/dependencyinjection/dicontainer.php

@@ -28,6 +28,7 @@ use OC;
 use OC\AppFramework\Http;
 use OC\AppFramework\Http\Request;
 use OC\AppFramework\Http\Dispatcher;
+use OC\AppFramework\Http\Output;
 use OC\AppFramework\Core\API;
 use OC\AppFramework\Middleware\MiddlewareDispatcher;
 use OC\AppFramework\Middleware\Security\SecurityMiddleware;
@@ -69,6 +70,10 @@ class DIContainer extends SimpleContainer implements IAppContainer {
 			return $this->getServer()->getAppManager();
 		});
 
+		$this->registerService('OCP\\AppFramework\\Http\\IOutput', function($c){
+			return new Output();
+		});
+
 		$this->registerService('OCP\\IAvatarManager', function($c) {
 			return $this->getServer()->getAvatarManager();
 		});

lib/private/appframework/http/dispatcher.php

@@ -100,17 +100,15 @@ class Dispatcher {
 		$response = $this->middlewareDispatcher->afterController(
 			$controller, $methodName, $response);
 
-		// get the output which should be printed and run the after output
-		// middleware to modify the response
-		$output = $response->render();
-		$out[3] = $this->middlewareDispatcher->beforeOutput(
-			$controller, $methodName, $output);
-
 		// depending on the cache object the headers need to be changed
 		$out[0] = $this->protocol->getStatusHeader($response->getStatus(),
 			$response->getLastModified(), $response->getETag());
 		$out[1] = array_merge($response->getHeaders());
 		$out[2] = $response->getCookies();
+		$out[3] = $this->middlewareDispatcher->beforeOutput(
+			$controller, $methodName, $response->render()
+		);
+		$out[4] = $response;
 
 		return $out;
 	}

lib/private/appframework/http/output.php

@@ -0,0 +1,70 @@
+<?php
+/**
+ * @author Bernhard Posselt
+ * @copyright 2015 Bernhard Posselt <dev@bernhard-posselt.com>
+ *
+ * This file is licensed under the Affero General Public License version 3 or
+ * later.
+ * See the COPYING-README file.
+ */
+
+namespace OC\AppFramework\Http;
+
+use OCP\AppFramework\Http\IOutput;
+
+/**
+ * Very thin wrapper class to make output testable
+ */
+class Output implements IOutput {
+
+	/**
+	 * @param string $out
+	 */
+	public function setOutput($out) {
+		print($out);
+	}
+
+	/**
+	 * @param string $path
+	 *
+	 * @return bool false if an error occured
+	 */
+	public function setReadfile($path) {
+		return @readfile($path);
+	}
+
+	/**
+	 * @param string $header
+	 */
+	public function setHeader($header) {
+		header($header);
+	}
+
+	/**
+	 * @param int $code sets the http status code
+	 */
+	public function setHttpResponseCode($code) {
+		http_response_code($code);
+	}
+
+	/**
+	 * @return int returns the current http response code
+	 */
+	public function getHttpResponseCode() {
+		return http_response_code();
+	}
+
+	/**
+	 * @param string $name
+	 * @param string $value
+	 * @param int $expire
+	 * @param string $path
+	 * @param string $domain
+	 * @param bool $secure
+	 * @param bool $httponly
+	 */
+	public function setCookie($name, $value, $expire, $path, $domain, $secure, $httponly) {
+		setcookie($name, $value, $expire, $path, $domain, $secure, $httponly);
+	}
+
+}

lib/public/appframework/http/icallbackresponse.php

@@ -0,0 +1,28 @@
+<?php
+/**
+ * @author Bernhard Posselt
+ * @copyright 2015 Bernhard Posselt <dev@bernhard-posselt.com>
+ *
+ * This file is licensed under the Affero General Public License version 3 or
+ * later.
+ * See the COPYING-README file.
+ */
+
+namespace OCP\AppFramework\Http;
+
+
+/**
+ * Interface ICallbackResponse
+ *
+ * @package OCP\AppFramework\Http
+ */
+interface ICallbackResponse {
+
+	/**
+	 * Outputs the content that should be printed
+	 *
+	 * @param IOutput a small wrapper that handles output
+	 */
+	function callback(IOutput $output);
+
+}

lib/public/appframework/http/ioutput.php

@@ -0,0 +1,57 @@
+<?php
+/**
+ * @author Bernhard Posselt
+ * @copyright 2015 Bernhard Posselt <dev@bernhard-posselt.com>
+ *
+ * This file is licensed under the Affero General Public License version 3 or
+ * later.
+ * See the COPYING-README file.
+ */
+
+namespace OCP\AppFramework\Http;
+
+
+/**
+ * Very thin wrapper class to make output testable
+ */
+interface IOutput {
+
+	/**
+	 * @param string $out
+	 */
+	public function setOutput($out);
+
+	/**
+	 * @param string $path
+	 *
+	 * @return bool false if an error occured
+	 */
+	public function setReadfile($path);
+
+	/**
+	 * @param string $header
+	 */
+	public function setHeader($header);
+
+	/**
+	 * @return int returns the current http response code
+	 */
+	public function getHttpResponseCode();
+
+	/**
+	 * @param int $code sets the http status code
+	 */
+	public function setHttpResponseCode($code);
+
+	/**
+	 * @param string $name
+	 * @param string $value
+	 * @param int $expire
+	 * @param string $path
+	 * @param string $domain
+	 * @param bool $secure
+	 * @param bool $httponly
+	 */
+	public function setCookie($name, $value, $expire, $path, $domain, $secure, $httponly);
+
+}

lib/public/appframework/http/streamresponse.php

@@ -0,0 +1,48 @@
+<?php
+/**
+ * @author Bernhard Posselt
+ * @copyright 2015 Bernhard Posselt <dev@bernhard-posselt.com>
+ *
+ * This file is licensed under the Affero General Public License version 3 or
+ * later.
+ * See the COPYING-README file.
+ */
+
+namespace OCP\AppFramework\Http;
+
+use OCP\AppFramework\Http;
+
+/**
+ * Class StreamResponse
+ *
+ * @package OCP\AppFramework\Http
+ */
+class StreamResponse extends Response implements ICallbackResponse {
+	/** @var string */
+	private $filePath;
+
+	/**
+	 * @param string $filePath the path to the file which should be streamed
+	 */
+	public function __construct ($filePath) {
+		$this->filePath = $filePath;
+	}
+
+
+	/**
+	 * Streams the file using readfile
+	 *
+	 * @param IOutput a small wrapper that handles output
+	 */
+	public function callback (IOutput $output) {
+		// handle caching
+		if ($output->getHttpResponseCode() !== Http::STATUS_NOT_MODIFIED) {
+			if (!file_exists($this->filePath)) {
+				$output->setHttpResponseCode(Http::STATUS_NOT_FOUND);
+			} elseif ($output->setReadfile($this->filePath) === false) {
+				$output->setHttpResponseCode(Http::STATUS_BAD_REQUEST);
+			}
+		}
+	}
+
+}

tests/lib/appframework/AppTest.php

@@ -24,6 +24,9 @@
 
 namespace OC\AppFramework;
 
+use OCP\AppFramework\Http\Response;
+
+
 function rrmdir($directory) {
 	$files = array_diff(scandir($directory), array('.','..'));
 	foreach ($files as $file) {
@@ -36,9 +39,11 @@ function rrmdir($directory) {
 	return rmdir($directory);
 }
 
+
 class AppTest extends \Test\TestCase {
 
 	private $container;
+	private $io;
 	private $api;
 	private $controller;
 	private $dispatcher;
@@ -62,6 +67,7 @@ class AppTest extends \Test\TestCase {
 			->disableOriginalConstructor()
 			->getMock();
 
+		$this->io = $this->getMockBuilder('OCP\\AppFramework\\Http\\IOutput')->getMock();
 
 		$this->headers = array('key' => 'value');
 		$this->output = 'hi';
@@ -70,6 +76,7 @@ class AppTest extends \Test\TestCase {
 
 		$this->container[$this->controllerName] = $this->controller;
 		$this->container['Dispatcher'] = $this->dispatcher;
+		$this->container['OCP\\AppFramework\\Http\\IOutput'] = $this->io;
 		$this->container['urlParams'] = array();
 
 		$this->appPath = __DIR__ . '/../../../apps/namespacetestapp/appinfo';
@@ -86,14 +93,15 @@ class AppTest extends \Test\TestCase {
 
 
 	public function testControllerNameAndMethodAreBeingPassed(){
-		$return = array(null, array(), array(), null);
+		$return = array(null, array(), array(), null, new Response());
 		$this->dispatcher->expects($this->once())
 			->method('dispatch')
 			->with($this->equalTo($this->controller),
 				$this->equalTo($this->controllerMethod))
 			->will($this->returnValue($return));
 
-		$this->expectOutputString('');
+		$this->io->expects($this->never())
+			->method('setOutput');
 
 		App::main($this->controllerName, $this->controllerMethod,
 			$this->container);
@@ -122,26 +130,34 @@ class AppTest extends \Test\TestCase {
 		rrmdir($this->appPath);
 	}
 
-	/*
-	FIXME: this complains about shit headers which are already sent because
-	of the content length. Would be cool if someone could fix this
 
 	public function testOutputIsPrinted(){
-		$return = array(null, array(), $this->output);
+		$return = [null, [], [], $this->output, new Response()];
 		$this->dispatcher->expects($this->once())
 			->method('dispatch')
 			->with($this->equalTo($this->controller),
 				$this->equalTo($this->controllerMethod))
 			->will($this->returnValue($return));
-
+		$this->io->expects($this->once())
-		$this->expectOutputString($this->output);
+			->method('setOutput')
-
+			->with($this->equalTo($this->output));
-		App::main($this->controllerName, $this->controllerMethod, array(),
+		App::main($this->controllerName, $this->controllerMethod, $this->container, []);
-			$this->container);
 	}
-	*/
 
-	// FIXME: if someone manages to test the headers output, I'd be grateful
 
+	public function testCallbackIsCalled(){
+		$mock = $this->getMockBuilder('OCP\AppFramework\Http\ICallbackResponse')
+			->getMock();
+
+		$return = [null, [], [], $this->output, $mock];
+		$this->dispatcher->expects($this->once())
+			->method('dispatch')
+			->with($this->equalTo($this->controller),
+				$this->equalTo($this->controllerMethod))
+			->will($this->returnValue($return));
+		$mock->expects($this->once())
+			->method('callback');
+		App::main($this->controllerName, $this->controllerMethod, $this->container, []);
+	}
 
 }

tests/lib/appframework/http/StreamResponseTest.php

@@ -0,0 +1,99 @@
+<?php
+
+/**
+ * ownCloud - App Framework
+ *
+ * @author Bernhard Posselt
+ * @copyright 2015 Bernhard Posselt <dev@bernhard-posselt.com>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU AFFERO GENERAL PUBLIC LICENSE
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or any later version.
+ *
+ * This library 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 library.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+
+namespace OC\AppFramework\Http;
+
+
+use OCP\AppFramework\Http\StreamResponse;
+use OCP\AppFramework\Http;
+
+
+class StreamResponseTest extends \Test\TestCase {
+
+	/** @var IOutput */
+	private $output;
+
+	protected function setUp() {
+		parent::setUp();
+		$this->output = $this->getMock('OCP\\AppFramework\\Http\\IOutput');
+	}
+
+	public function testOutputNotModified(){
+		$path = __FILE__;
+		$this->output->expects($this->once())
+			->method('getHttpResponseCode')
+			->will($this->returnValue(Http::STATUS_NOT_MODIFIED));
+		$this->output->expects($this->never())
+			->method('setReadfile');
+		$response = new StreamResponse($path);
+
+		$response->callback($this->output);
+	}
+
+	public function testOutputOk(){
+		$path = __FILE__;
+		$this->output->expects($this->once())
+			->method('getHttpResponseCode')
+			->will($this->returnValue(Http::STATUS_OK));
+		$this->output->expects($this->once())
+			->method('setReadfile')
+			->with($this->equalTo($path))
+			->will($this->returnValue(true));
+		$response = new StreamResponse($path);
+
+		$response->callback($this->output);
+	}
+
+	public function testOutputNotFound(){
+		$path = __FILE__ . 'test';
+		$this->output->expects($this->once())
+			->method('getHttpResponseCode')
+			->will($this->returnValue(Http::STATUS_OK));
+		$this->output->expects($this->never())
+			->method('setReadfile');
+		$this->output->expects($this->once())
+			->method('setHttpResponseCode')
+			->with($this->equalTo(Http::STATUS_NOT_FOUND));
+		$response = new StreamResponse($path);
+
+		$response->callback($this->output);
+	}
+
+	public function testOutputReadFileError(){
+		$path = __FILE__;
+		$this->output->expects($this->once())
+			->method('getHttpResponseCode')
+			->will($this->returnValue(Http::STATUS_OK));
+		$this->output->expects($this->once())
+			->method('setReadfile')
+			->will($this->returnValue(false));
+		$this->output->expects($this->once())
+			->method('setHttpResponseCode')
+			->with($this->equalTo(Http::STATUS_BAD_REQUEST));
+		$response = new StreamResponse($path);
+
+		$response->callback($this->output);
+	}
+
+}