]> source.dussan.org Git - nextcloud-server.git/commitdiff
AppFramework StreamResponse
authorBernhard Posselt <dev@bernhard-posselt.com>
Thu, 22 Jan 2015 19:26:46 +0000 (20:26 +0100)
committerBernhard Posselt <dev@bernhard-posselt.com>
Fri, 27 Feb 2015 14:42:33 +0000 (15:42 +0100)
First stab at the StreamResponse, see #12988

The idea is to use an interface ICallbackResponse (I'm not 100% happy with the name yet, suggestions?) that allow the response to output things in its own way, for instance stream the file using readfile

Unittests are atm lacking, plan is to

check if a mock of ICallbackResponse will be used by calling its callback (also unhappy with this name) method
Usage is:

$response = new StreamResponse('path/to/file');

rename io to output, add additional methods and handle error and not modified cases when using StreamResponse

fix indention and uppercasing, also handle forbidden cases

fix indention

fix indention

no forbidden, figuring out if a file is really readable is too complicated to get to work across OSes and streams

remove useless import

remove useless import

fix intendation

lib/private/appframework/app.php
lib/private/appframework/dependencyinjection/dicontainer.php
lib/private/appframework/http/dispatcher.php
lib/private/appframework/http/output.php [new file with mode: 0644]
lib/public/appframework/http/icallbackresponse.php [new file with mode: 0644]
lib/public/appframework/http/ioutput.php [new file with mode: 0644]
lib/public/appframework/http/streamresponse.php [new file with mode: 0644]
tests/lib/appframework/AppTest.php
tests/lib/appframework/http/StreamResponseTest.php [new file with mode: 0644]

index 537f10255a37864f3ff9f8a81de42085a1e79237..6d54b931d5a815ad1ceaf1a675558b23f1e1a455 100644 (file)
 
 namespace OC\AppFramework;
 
-use \OC_App;
-use \OC\AppFramework\DependencyInjection\DIContainer;
-use \OCP\AppFramework\QueryException;
+use OC_App;
+use OC\AppFramework\DependencyInjection\DIContainer;
+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) =
-                       $dispatcher->dispatch($controller, $methodName);
+               list(
+                       $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)) {
-                       header('Content-Length: ' . strlen($output));
-                       print($output);
+               if ($response instanceof ICallbackResponse) {
+                       $response->callback($io);
+               } else if(!is_null($output)) {
+                       $io->setHeader('Content-Length: ' . strlen($output));
+                       $io->setOutput($output);
                }
 
        }
index 4229b251e294d480f2d200f3a5eeb4c2fd09e8f5..e88177c63980ff05829447cc28a1b0137abb72d2 100644 (file)
@@ -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();
                });
index 24540ef3c94814229ae02f21e18ccd6ec38abe3d..910e9c32ed4e62c5506005fd42ce7ee9641e2d83 100644 (file)
@@ -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;
        }
diff --git a/lib/private/appframework/http/output.php b/lib/private/appframework/http/output.php
new file mode 100644 (file)
index 0000000..808f1ec
--- /dev/null
@@ -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);
+       }
+
+}
diff --git a/lib/public/appframework/http/icallbackresponse.php b/lib/public/appframework/http/icallbackresponse.php
new file mode 100644 (file)
index 0000000..4a392ed
--- /dev/null
@@ -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);
+
+}
diff --git a/lib/public/appframework/http/ioutput.php b/lib/public/appframework/http/ioutput.php
new file mode 100644 (file)
index 0000000..191f843
--- /dev/null
@@ -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);
+
+}
diff --git a/lib/public/appframework/http/streamresponse.php b/lib/public/appframework/http/streamresponse.php
new file mode 100644 (file)
index 0000000..870eb95
--- /dev/null
@@ -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);
+                       }
+               }
+       }
+
+}
index e60f3439f23535f71965673998c7acd2f8e30187..05190ca09b5852e78b8c167206cdadbdc875d7c7 100644 (file)
@@ -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->expectOutputString($this->output);
-
-               App::main($this->controllerName, $this->controllerMethod, array(),
-                       $this->container);
+               $this->io->expects($this->once())
+                       ->method('setOutput')
+                       ->with($this->equalTo($this->output));
+               App::main($this->controllerName, $this->controllerMethod, $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, []);
+       }
 
 }
diff --git a/tests/lib/appframework/http/StreamResponseTest.php b/tests/lib/appframework/http/StreamResponseTest.php
new file mode 100644 (file)
index 0000000..4c47ecf
--- /dev/null
@@ -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);
+       }
+
+}