From 0696099bad56727d96c60f6221fe02dc7c71f511 Mon Sep 17 00:00:00 2001 From: Bernhard Posselt Date: Tue, 28 Oct 2014 16:34:04 +0100 Subject: [PATCH] add dataresponse fix docstrings adjust copyright date another copyright date update another header update implement third headers argument, fix indention, fix docstrings fix docstrings --- lib/private/appframework/http/dispatcher.php | 5 +- lib/public/appframework/controller.php | 14 ++- lib/public/appframework/http/dataresponse.php | 79 +++++++++++++++++ lib/public/appframework/http/response.php | 14 ++- .../controller/ControllerTest.php | 22 +++++ .../appframework/http/DataResponseTest.php | 87 +++++++++++++++++++ .../lib/appframework/http/DispatcherTest.php | 74 ++++++++++++---- tests/lib/appframework/http/ResponseTest.php | 26 ++++-- 8 files changed, 292 insertions(+), 29 deletions(-) create mode 100644 lib/public/appframework/http/dataresponse.php create mode 100644 tests/lib/appframework/http/DataResponseTest.php diff --git a/lib/private/appframework/http/dispatcher.php b/lib/private/appframework/http/dispatcher.php index 7f2717951a..29a661d574 100644 --- a/lib/private/appframework/http/dispatcher.php +++ b/lib/private/appframework/http/dispatcher.php @@ -30,6 +30,7 @@ use \OC\AppFramework\Utility\ControllerMethodReflector; use OCP\AppFramework\Controller; use OCP\AppFramework\Http\Response; +use OCP\AppFramework\Http\DataResponse; use OCP\IRequest; @@ -154,8 +155,8 @@ class Dispatcher { $response = call_user_func_array(array($controller, $methodName), $arguments); - // format response if not of type response - if(!($response instanceof Response)) { + // format response + if($response instanceof DataResponse || !($response instanceof Response)) { // get format from the url format or request format parameter $format = $this->request->getParam('format'); diff --git a/lib/public/appframework/controller.php b/lib/public/appframework/controller.php index b22eb73343..398304e6fe 100644 --- a/lib/public/appframework/controller.php +++ b/lib/public/appframework/controller.php @@ -29,6 +29,7 @@ namespace OCP\AppFramework; use OCP\AppFramework\Http\TemplateResponse; use OCP\AppFramework\Http\JSONResponse; +use OCP\AppFramework\Http\DataResponse; use OCP\IRequest; @@ -63,8 +64,17 @@ abstract class Controller { // default responders $this->responders = array( - 'json' => function ($response) { - return new JSONResponse($response); + 'json' => function ($data) { + if ($data instanceof DataResponse) { + $response = new JSONResponse( + $data->getData(), + $data->getStatus() + ); + $response->setHeaders($data->getHeaders()); + return $response; + } else { + return new JSONResponse($data); + } } ); } diff --git a/lib/public/appframework/http/dataresponse.php b/lib/public/appframework/http/dataresponse.php new file mode 100644 index 0000000000..5c21de325e --- /dev/null +++ b/lib/public/appframework/http/dataresponse.php @@ -0,0 +1,79 @@ + + * + * 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 . + * + */ + +/** + * Public interface of ownCloud for apps to use. + * AppFramework\HTTP\DataResponse class + */ + +namespace OCP\AppFramework\Http; + +use OCP\AppFramework\Http; + +/** + * A generic DataResponse class that is used to return generic data responses + * for responders to transform + */ +class DataResponse extends Response { + + /** + * response data + * @var array|object + */ + protected $data; + + + /** + * @param array|object $data the object or array that should be transformed + * @param int $statusCode the Http status code, defaults to 200 + * @param array $headers additional key value based headers + */ + public function __construct($data=array(), $statusCode=Http::STATUS_OK, + array $headers=array()) { + $this->data = $data; + $this->setStatus($statusCode); + $this->setHeaders(array_merge($this->getHeaders(), $headers)); + } + + + /** + * Sets values in the data json array + * @param array|object $data an array or object which will be transformed + * @return DataResponse Reference to this object + */ + public function setData($data){ + $this->data = $data; + + return $this; + } + + + /** + * Used to get the set parameters + * @return array the data + */ + public function getData(){ + return $this->data; + } + + +} diff --git a/lib/public/appframework/http/response.php b/lib/public/appframework/http/response.php index 20e936bb86..354911fee2 100644 --- a/lib/public/appframework/http/response.php +++ b/lib/public/appframework/http/response.php @@ -93,7 +93,7 @@ class Response { */ public function addHeader($name, $value) { $name = trim($name); // always remove leading and trailing whitespace - // to be able to reliably check for security + // to be able to reliably check for security // headers if(is_null($value)) { @@ -106,6 +106,18 @@ class Response { } + /** + * Set the headers + * @param array key value header pairs + * @return Response Reference to this object + */ + public function setHeaders($headers) { + $this->headers = $headers; + + return $this; + } + + /** * Returns the set headers * @return array the headers diff --git a/tests/lib/appframework/controller/ControllerTest.php b/tests/lib/appframework/controller/ControllerTest.php index e97ec54893..0de94ff5b7 100644 --- a/tests/lib/appframework/controller/ControllerTest.php +++ b/tests/lib/appframework/controller/ControllerTest.php @@ -27,6 +27,7 @@ namespace OCP\AppFramework; use OC\AppFramework\Http\Request; use OCP\AppFramework\Http\TemplateResponse; use OCP\AppFramework\Http\JSONResponse; +use OCP\AppFramework\Http\DataResponse; class ChildController extends Controller { @@ -45,6 +46,12 @@ class ChildController extends Controller { return $in; } + + public function customDataResponse($in) { + $response = new DataResponse($in, 300); + $response->addHeader('test', 'something'); + return $response; + } }; class ControllerTest extends \PHPUnit_Framework_TestCase { @@ -161,6 +168,21 @@ class ControllerTest extends \PHPUnit_Framework_TestCase { } + public function testFormatDataResponseJSON() { + $expectedHeaders = array( + 'test' => 'something', + 'Cache-Control' => 'no-cache, must-revalidate' + ); + + $response = $this->controller->customDataResponse(array('hi')); + $response = $this->controller->buildResponse($response, 'json'); + + $this->assertEquals(array('hi'), $response->getData()); + $this->assertEquals(300, $response->getStatus()); + $this->assertEquals($expectedHeaders, $response->getHeaders()); + } + + public function testCustomFormatter() { $response = $this->controller->custom('hi'); $response = $this->controller->buildResponse($response, 'json'); diff --git a/tests/lib/appframework/http/DataResponseTest.php b/tests/lib/appframework/http/DataResponseTest.php new file mode 100644 index 0000000000..961327c978 --- /dev/null +++ b/tests/lib/appframework/http/DataResponseTest.php @@ -0,0 +1,87 @@ + + * + * 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 . + * + */ + + +namespace OC\AppFramework\Http; + + +use OCP\AppFramework\Http\DataResponse; +use OCP\AppFramework\Http; + + +class DataResponseTest extends \PHPUnit_Framework_TestCase { + + /** + * @var DataResponse + */ + private $response; + + protected function setUp() { + $this->response = new DataResponse(); + } + + + public function testSetData() { + $params = array('hi', 'yo'); + $this->response->setData($params); + + $this->assertEquals(array('hi', 'yo'), $this->response->getData()); + } + + + public function testConstructorAllowsToSetData() { + $data = array('hi'); + $code = 300; + $response = new DataResponse($data, $code); + + $this->assertEquals($data, $response->getData()); + $this->assertEquals($code, $response->getStatus()); + } + + + public function testConstructorAllowsToSetHeaders() { + $data = array('hi'); + $code = 300; + $headers = array('test' => 'something'); + $response = new DataResponse($data, $code, $headers); + + $expectedHeaders = array('Cache-Control' => 'no-cache, must-revalidate'); + $expectedHeaders = array_merge($expectedHeaders, $headers); + + $this->assertEquals($data, $response->getData()); + $this->assertEquals($code, $response->getStatus()); + $this->assertEquals($expectedHeaders, $response->getHeaders()); + } + + + public function testChainability() { + $params = array('hi', 'yo'); + $this->response->setData($params) + ->setStatus(Http::STATUS_NOT_FOUND); + + $this->assertEquals(Http::STATUS_NOT_FOUND, $this->response->getStatus()); + $this->assertEquals(array('hi', 'yo'), $this->response->getData()); + } + + +} diff --git a/tests/lib/appframework/http/DispatcherTest.php b/tests/lib/appframework/http/DispatcherTest.php index 9d5ec09a29..f082ddc8b3 100644 --- a/tests/lib/appframework/http/DispatcherTest.php +++ b/tests/lib/appframework/http/DispatcherTest.php @@ -28,6 +28,7 @@ use OC\AppFramework\Middleware\MiddlewareDispatcher; use OC\AppFramework\Utility\ControllerMethodReflector; use OCP\AppFramework\Http; use OCP\AppFramework\Http\JSONResponse; +use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\Controller; @@ -46,6 +47,18 @@ class TestController extends Controller { }); return array($int, $bool, $test, $test2); } + + + /** + * @param int $int + * @param bool $bool + */ + public function execDataResponse($int, $bool, $test=4, $test2=1) { + return new DataResponse(array( + 'text' => array($int, $bool, $test, $test2) + )); + } + } @@ -84,7 +97,7 @@ class DispatcherTest extends \PHPUnit_Framework_TestCase { $this->controller = $this->getMock( '\OCP\AppFramework\Controller', array($this->controllerMethod), array($app, $request)); - + $this->request = $this->getMockBuilder( '\OC\AppFramework\Http\Request') ->disableOriginalConstructor() @@ -96,7 +109,7 @@ class DispatcherTest extends \PHPUnit_Framework_TestCase { $this->http, $this->middlewareDispatcher, $this->reflector, $this->request ); - + $this->response = $this->getMockBuilder( '\OCP\AppFramework\Http\Response') ->disableOriginalConstructor() @@ -111,7 +124,7 @@ class DispatcherTest extends \PHPUnit_Framework_TestCase { * @param string $out * @param string $httpHeaders */ - private function setMiddlewareExpectations($out=null, + private function setMiddlewareExpectations($out=null, $httpHeaders=null, $responseHeaders=array(), $ex=false, $catchEx=true) { @@ -119,20 +132,20 @@ class DispatcherTest extends \PHPUnit_Framework_TestCase { $exception = new \Exception(); $this->middlewareDispatcher->expects($this->once()) ->method('beforeController') - ->with($this->equalTo($this->controller), + ->with($this->equalTo($this->controller), $this->equalTo($this->controllerMethod)) ->will($this->throwException($exception)); if($catchEx) { $this->middlewareDispatcher->expects($this->once()) ->method('afterException') - ->with($this->equalTo($this->controller), + ->with($this->equalTo($this->controller), $this->equalTo($this->controllerMethod), $this->equalTo($exception)) ->will($this->returnValue($this->response)); } else { $this->middlewareDispatcher->expects($this->once()) ->method('afterException') - ->with($this->equalTo($this->controller), + ->with($this->equalTo($this->controller), $this->equalTo($this->controllerMethod), $this->equalTo($exception)) ->will($this->returnValue(null)); @@ -141,7 +154,7 @@ class DispatcherTest extends \PHPUnit_Framework_TestCase { } else { $this->middlewareDispatcher->expects($this->once()) ->method('beforeController') - ->with($this->equalTo($this->controller), + ->with($this->equalTo($this->controller), $this->equalTo($this->controllerMethod)); $this->controller->expects($this->once()) ->method($this->controllerMethod) @@ -165,38 +178,38 @@ class DispatcherTest extends \PHPUnit_Framework_TestCase { ->will($this->returnValue($responseHeaders)); $this->http->expects($this->once()) ->method('getStatusHeader') - ->with($this->equalTo(Http::STATUS_OK), + ->with($this->equalTo(Http::STATUS_OK), $this->equalTo($this->lastModified), $this->equalTo($this->etag)) ->will($this->returnValue($httpHeaders)); - + $this->middlewareDispatcher->expects($this->once()) ->method('afterController') - ->with($this->equalTo($this->controller), + ->with($this->equalTo($this->controller), $this->equalTo($this->controllerMethod), $this->equalTo($this->response)) ->will($this->returnValue($this->response)); $this->middlewareDispatcher->expects($this->once()) ->method('afterController') - ->with($this->equalTo($this->controller), + ->with($this->equalTo($this->controller), $this->equalTo($this->controllerMethod), $this->equalTo($this->response)) ->will($this->returnValue($this->response)); $this->middlewareDispatcher->expects($this->once()) ->method('beforeOutput') - ->with($this->equalTo($this->controller), + ->with($this->equalTo($this->controller), $this->equalTo($this->controllerMethod), $this->equalTo($out)) - ->will($this->returnValue($out)); + ->will($this->returnValue($out)); } public function testDispatcherReturnsArrayWith2Entries() { $this->setMiddlewareExpectations(); - $response = $this->dispatcher->dispatch($this->controller, + $response = $this->dispatcher->dispatch($this->controller, $this->controllerMethod); $this->assertNull($response[0]); $this->assertEquals(array(), $response[1]); @@ -210,7 +223,7 @@ class DispatcherTest extends \PHPUnit_Framework_TestCase { $responseHeaders = array('hell' => 'yeah'); $this->setMiddlewareExpectations($out, $httpHeaders, $responseHeaders); - $response = $this->dispatcher->dispatch($this->controller, + $response = $this->dispatcher->dispatch($this->controller, $this->controllerMethod); $this->assertEquals($httpHeaders, $response[0]); @@ -227,9 +240,9 @@ class DispatcherTest extends \PHPUnit_Framework_TestCase { $out = 'yo'; $httpHeaders = 'Http'; $responseHeaders = array('hell' => 'yeah'); - $this->setMiddlewareExpectations($out, $httpHeaders, $responseHeaders, true); + $this->setMiddlewareExpectations($out, $httpHeaders, $responseHeaders, true); - $response = $this->dispatcher->dispatch($this->controller, + $response = $this->dispatcher->dispatch($this->controller, $this->controllerMethod); $this->assertEquals($httpHeaders, $response[0]); @@ -249,7 +262,7 @@ class DispatcherTest extends \PHPUnit_Framework_TestCase { $this->setMiddlewareExpectations($out, $httpHeaders, $responseHeaders, true, false); $this->setExpectedException('\Exception'); - $response = $this->dispatcher->dispatch($this->controller, + $response = $this->dispatcher->dispatch($this->controller, $this->controllerMethod); } @@ -342,6 +355,31 @@ class DispatcherTest extends \PHPUnit_Framework_TestCase { } + public function testResponseTransformsDataResponse() { + $this->request = new Request(array( + 'post' => array( + 'int' => '3', + 'bool' => 'false' + ), + 'urlParams' => array( + 'format' => 'json' + ), + 'method' => 'GET' + )); + $this->dispatcher = new Dispatcher( + $this->http, $this->middlewareDispatcher, $this->reflector, + $this->request + ); + $controller = new TestController('app', $this->request); + + // reflector is supposed to be called once + $this->dispatcherPassthrough(); + $response = $this->dispatcher->dispatch($controller, 'execDataResponse'); + + $this->assertEquals('{"text":[3,false,4,1]}', $response[2]); + } + + public function testResponseTransformedByAcceptHeader() { $this->request = new Request(array( 'post' => array( diff --git a/tests/lib/appframework/http/ResponseTest.php b/tests/lib/appframework/http/ResponseTest.php index e83fe9e2d8..b1dddd9ebc 100644 --- a/tests/lib/appframework/http/ResponseTest.php +++ b/tests/lib/appframework/http/ResponseTest.php @@ -48,10 +48,24 @@ class ResponseTest extends \PHPUnit_Framework_TestCase { } + function testSetHeaders(){ + $expected = array( + 'Last-Modified' => 1, + 'ETag' => 3, + 'Something-Else' => 'hi' + ); + + $this->childResponse->setHeaders($expected); + $headers = $this->childResponse->getHeaders(); + + $this->assertEquals($expected, $headers); + } + + public function testAddHeaderValueNullDeletesIt(){ $this->childResponse->addHeader('hello', 'world'); $this->childResponse->addHeader('hello', null); - $this->assertEquals(1, count($this->childResponse->getHeaders())); + $this->assertEquals(1, count($this->childResponse->getHeaders())); } @@ -93,18 +107,18 @@ class ResponseTest extends \PHPUnit_Framework_TestCase { public function testCacheSecondsZero() { $this->childResponse->cacheFor(0); - + $headers = $this->childResponse->getHeaders(); - $this->assertEquals('no-cache, must-revalidate', $headers['Cache-Control']); + $this->assertEquals('no-cache, must-revalidate', $headers['Cache-Control']); } public function testCacheSeconds() { $this->childResponse->cacheFor(33); - + $headers = $this->childResponse->getHeaders(); - $this->assertEquals('max-age=33, must-revalidate', - $headers['Cache-Control']); + $this->assertEquals('max-age=33, must-revalidate', + $headers['Cache-Control']); }