From 0fa1411d2a33c1f51cbd9f412d9f961f96e350c8 Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Fri, 4 Jan 2013 14:33:23 -0600 Subject: [PATCH 1/2] Provide support for more HTTP methods in the AbstractRestfulController - Adds explicit support for OPTIONS, PATCH, and HEAD - Adds ability to register handler methods for custom HTTP methods - Refactors: - DRY up identifier retrieval -- added getIdentifier() method - DRY up parsing of content body -- added processBodyContent() - Moved RESTful method handling out of conditional --- .../Controller/AbstractRestfulController.php | 289 +++++++++++++----- .../Mvc/Controller/RestfulControllerTest.php | 81 ++++- .../Mvc/Controller/TestAsset/Request.php | 33 ++ .../TestAsset/RestfulTestController.php | 76 +++-- 4 files changed, 392 insertions(+), 87 deletions(-) create mode 100644 tests/ZendTest/Mvc/Controller/TestAsset/Request.php diff --git a/library/Zend/Mvc/Controller/AbstractRestfulController.php b/library/Zend/Mvc/Controller/AbstractRestfulController.php index eeab5dba5e4..d15c001698c 100644 --- a/library/Zend/Mvc/Controller/AbstractRestfulController.php +++ b/library/Zend/Mvc/Controller/AbstractRestfulController.php @@ -44,11 +44,27 @@ abstract class AbstractRestfulController extends AbstractController ); /** - * Return list of resources + * Map of custom HTTP methods and their handlers + * + * @var array + */ + protected $customHttpMethodsMap = array(); + + /** + * Create a new resource * + * @param mixed $data * @return mixed */ - abstract public function getList(); + abstract public function create($data); + + /** + * Delete an existing resource + * + * @param mixed $id + * @return mixed + */ + abstract public function delete($id); /** * Return single resource @@ -59,29 +75,38 @@ abstract public function getList(); abstract public function get($id); /** - * Create a new resource + * Retrieve HEAD metadata for the resource * - * @param mixed $data + * @param null|mixed $id * @return mixed */ - abstract public function create($data); + abstract public function head($id = null); /** - * Update an existing resource + * Respond to the OPTIONS method + * + * Typically, set the Allow header with allowed HTTP methods, and + * return the response. * - * @param mixed $id - * @param mixed $data * @return mixed */ - abstract public function update($id, $data); + abstract public function options(); /** - * Delete an existing resource + * Return list of resources + * + * @return mixed + */ + abstract public function getList(); + + /** + * Update an existing resource * * @param mixed $id + * @param mixed $data * @return mixed */ - abstract public function delete($id); + abstract public function update($id, $data); /** * Basic functionality for when a page is not available @@ -140,7 +165,9 @@ public function onDispatch(MvcEvent $e) } $request = $e->getRequest(); - $action = $routeMatch->getParam('action', false); + + // Was an "action" requested? + $action = $routeMatch->getParam('action', false); if ($action) { // Handle arbitrary methods, ending in Action $method = static::getMethodFromAction($action); @@ -148,53 +175,86 @@ public function onDispatch(MvcEvent $e) $method = 'notFoundAction'; } $return = $this->$method(); - } else { - // RESTful methods - switch (strtolower($request->getMethod())) { - case 'get': - if (null !== $id = $routeMatch->getParam('id')) { - $action = 'get'; - $return = $this->get($id); - break; - } - if (null !== $id = $request->getQuery()->get('id')) { - $action = 'get'; - $return = $this->get($id); - break; - } - $action = 'getList'; - $return = $this->getList(); - break; - case 'post': - $action = 'create'; - $return = $this->processPostData($request); - break; - case 'put': - $action = 'update'; - $return = $this->processPutData($request, $routeMatch); - break; - case 'delete': - if (null === $id = $routeMatch->getParam('id')) { - if (! ($id = $request->getQuery()->get('id', false))) { - throw new Exception\DomainException( - 'Missing identifier'); - } + $e->setResult($return); + return $return; + } + + // RESTful methods + $method = strtolower($request->getMethod()); + switch ($method) { + // Custom HTTP methods (or custom overrides for standard methods) + case (isset($this->customHttpMethodsMap[$method])): + $callable = $this->customHttpMethodsMap[$method]; + $action = $method; + $return = call_user_func($callable, $e); + break; + // DELETE + case 'delete': + if (null === $id = $routeMatch->getParam('id')) { + if (! ($id = $request->getQuery()->get('id', false))) { + throw new Exception\DomainException( + 'Missing identifier'); } - $action = 'delete'; - $return = $this->delete($id); + } + $action = 'delete'; + $return = $this->delete($id); + break; + // GET + case 'get': + $id = $this->getIdentifier($routeMatch, $request); + if ($id) { + $action = 'get'; + $return = $this->get($id); break; - default: - throw new Exception\DomainException('Invalid HTTP method!'); - } - - $routeMatch->setParam('action', $action); + } + $action = 'getList'; + $return = $this->getList(); + break; + // HEAD + case 'head': + $id = $this->getIdentifier($routeMatch, $request); + if (!$id) { + $id = null; + } + $action = 'head'; + $this->head($id); + $response = $e->getResponse(); + $response->setContent(''); + $return = $response; + break; + // OPTIONS + case 'options': + $action = 'options'; + $this->options(); + $return = $e->getResponse(); + break; + // PATCH + case 'patch': + $id = $this->getIdentifier($routeMatch, $request); + if (!$id) { + throw new Exception\DomainException('Missing identifier'); + } + $data = $this->processBodyContent($request); + $action = 'patch'; + $return = $this->patch($id, $data); + break; + // POST + case 'post': + $action = 'create'; + $return = $this->processPostData($request); + break; + // PUT + case 'put': + $action = 'update'; + $return = $this->processPutData($request, $routeMatch); + break; + // All others... + default: + throw new Exception\DomainException('Invalid HTTP method!'); } - // Emit post-dispatch signal, passing: - // - return from method, request, response - // If a listener returns a response object, return it immediately + $routeMatch->setParam('action', $action); $e->setResult($return); - return $return; } @@ -207,11 +267,12 @@ public function onDispatch(MvcEvent $e) public function processPostData(Request $request) { if ($this->requestHasContentType($request, self::CONTENT_TYPE_JSON)) { - return $this->create(Json::decode($request->getContent())); + $data = Json::decode($request->getContent()); + } else { + $data = $request->getPost()->toArray(); } - return $this->create($request->getPost() - ->toArray()); + return $this->create($data); } /** @@ -224,20 +285,14 @@ public function processPostData(Request $request) */ public function processPutData(Request $request, $routeMatch) { - if (null === $id = $routeMatch->getParam('id')) { - if (! ($id = $request->getQuery()->get('id', false))) { - throw new Exception\DomainException('Missing identifier'); - } + $id = $this->getIdentifier($routeMatch, $request); + if (!$id) { + throw new Exception\DomainException('Missing identifier'); } - if ($this->requestHasContentType($request, self::CONTENT_TYPE_JSON)) { - return $this->update($id, Json::decode($request->getContent())); - } + $data = $this->processBodyContent($request); - $content = $request->getContent(); - parse_str($content, $parsedParams); - - return $this->update($id, $parsedParams); + return $this->update($id, $data); } /** @@ -262,4 +317,102 @@ public function requestHasContentType(Request $request, $contentType = '') return false; } + + /** + * Register a handler for a custom HTTP method + * + * This method allows you to handle arbitrary HTTP method types, mapping + * them to callables. Typically, these will be methods of the controller + * instance: e.g., array($this, 'foobar'). The typical place to register + * these is in your constructor. + * + * Additionally, as this map is checked prior to testing the standard HTTP + * methods, this is a way to override what methods will handle the standard + * HTTP methods. However, if you do this, you will have to retrieve the + * identifier and any request content manually. + * + * Callbacks will be passed the current MvcEvent instance. + * + * To retrieve the identifier, you can use "$id = + * $this->getIdentifier($routeMatch, $request)", + * passing the appropriate objects. + * + * To retrive the body content data, use "$data = $this->processBodyContent($request)"; + * that method will return a string, array, or, in the case of JSON, an object. + * + * @param string $method + * @param Callable $handler + * @return AbstractRestfulController + */ + public function addHttpMethodHandler($method, /* Callable */ $handler) + { + if (!is_callable($handler)) { + throw new Exception\InvalidArgumentException(sprintf( + 'Invalid HTTP method handler: must be a callable; received "%s"', + (is_object($handler) ? get_class($handler) : gettype($handler)) + )); + } + $method = strtolower($method); + $this->customHttpMethodsMap[$method] = $handler; + return $this; + } + + /** + * Retrieve the identifier, if any + * + * Attempts to see if an identifier was passed in either the URI or the + * query string, returning if if found. Otherwise, returns a boolean false. + * + * @param \Zend\Mvc\Router\RouteMatch $routeMatch + * @param Request $request + * @return false|mixed + */ + protected function getIdentifier($routeMatch, $request) + { + $id = $routeMatch->getParam('id', false); + if ($id) { + return $id; + } + + $id = $request->getQuery()->get('id', false); + if ($id) { + return $id; + } + + return false; + } + + /** + * Process the raw body content + * + * If the content-type indicates a JSON payload, the payload is immediately + * decoded and the data returned. Otherwise, the data is passed to + * parse_str(). If that function returns a single-member array with a key + * of "0", the method assumes that we have non-urlencoded content and + * returns the raw content; otherwise, the array created is returned. + * + * @param mixed $request + * @return object|string|array + */ + protected function processBodyContent($request) + { + $content = $request->getContent(); + + // JSON content? decode and return it. + if ($this->requestHasContentType($request, self::CONTENT_TYPE_JSON)) { + return Json::decode($content); + } + + parse_str($content, $parsedParams); + + // If parse_str fails to decode, or we have a single element with key + // 0, return the raw content. + if (!is_array($parsedParams) + || (1 == count($parsedParams) && isset($parsedParams[0])) + ) { + return $content; + } + + return $parsedParams; + } } diff --git a/tests/ZendTest/Mvc/Controller/RestfulControllerTest.php b/tests/ZendTest/Mvc/Controller/RestfulControllerTest.php index 12ab90ea9ea..1241f0a6a47 100644 --- a/tests/ZendTest/Mvc/Controller/RestfulControllerTest.php +++ b/tests/ZendTest/Mvc/Controller/RestfulControllerTest.php @@ -13,7 +13,6 @@ use PHPUnit_Framework_TestCase as TestCase; use stdClass; use Zend\EventManager\SharedEventManager; -use Zend\Http\Request; use Zend\Http\Response; use Zend\Mvc\MvcEvent; use Zend\Mvc\Router\RouteMatch; @@ -29,7 +28,8 @@ class RestfulControllerTest extends TestCase public function setUp() { $this->controller = new TestAsset\RestfulTestController(); - $this->request = new Request(); + $this->request = new TestAsset\Request(); + $this->response = new Response(); $this->routeMatch = new RouteMatch(array('controller' => 'controller-restful')); $this->event = new MvcEvent; $this->event->setRouteMatch($this->routeMatch); @@ -102,6 +102,83 @@ public function testDispatchInvokesDeleteMethodWhenNoActionPresentAndDeleteInvok $this->assertEquals('delete', $this->routeMatch->getParam('action')); } + public function testDispatchInvokesOptionsMethodWhenNoActionPresentAndOptionsInvoked() + { + $this->request->setMethod('OPTIONS'); + $result = $this->controller->dispatch($this->request, $this->response); + $this->assertSame($this->response, $result); + $this->assertEquals('options', $this->routeMatch->getParam('action')); + $headers = $result->getHeaders(); + $this->assertTrue($headers->has('Allow')); + $allow = $headers->get('Allow'); + $expected = explode(', ', 'GET, POST, PUT, DELETE, PATCH, HEAD, TRACE'); + sort($expected); + $test = explode(', ', $allow->getFieldValue()); + sort($test); + $this->assertEquals($expected, $test); + } + + public function testDispatchInvokesPatchMethodWhenNoActionPresentAndPatchInvokedWithIdentifier() + { + $entity = new stdClass; + $entity->name = 'foo'; + $entity->type = 'standard'; + $this->controller->entity = $entity; + $entity = array('name' => __FUNCTION__); + $string = http_build_query($entity); + $this->request->setMethod('PATCH') + ->setContent($string); + $this->routeMatch->setParam('id', 1); + $result = $this->controller->dispatch($this->request, $this->response); + $this->assertArrayHasKey('entity', $result); + $test = $result['entity']; + $this->assertArrayHasKey('id', $test); + $this->assertEquals(1, $test['id']); + $this->assertArrayHasKey('name', $test); + $this->assertEquals(__FUNCTION__, $test['name']); + $this->assertArrayHasKey('type', $test); + $this->assertEquals('standard', $test['type']); + $this->assertEquals('patch', $this->routeMatch->getParam('action')); + } + + public function testDispatchInvokesHeadMethodWhenNoActionPresentAndHeadInvokedWithoutIdentifier() + { + $entities = array( + new stdClass, + new stdClass, + new stdClass, + ); + $this->controller->entities = $entities; + $this->request->setMethod('HEAD'); + $result = $this->controller->dispatch($this->request, $this->response); + $this->assertSame($this->response, $result); + $content = $result->getContent(); + $this->assertEquals('', $content); + $this->assertEquals('head', $this->routeMatch->getParam('action')); + } + + public function testDispatchInvokesHeadMethodWhenNoActionPresentAndHeadInvokedWithIdentifier() + { + $entity = new stdClass; + $this->controller->entity = $entity; + $this->routeMatch->setParam('id', 1); + $this->request->setMethod('HEAD'); + $result = $this->controller->dispatch($this->request, $this->response); + $this->assertSame($this->response, $result); + $content = $result->getContent(); + $this->assertEquals('', $content); + $this->assertEquals('head', $this->routeMatch->getParam('action')); + } + + public function testAllowsRegisteringCustomHttpMethodsWithHandlers() + { + $this->controller->addHttpMethodHandler('DESCRIBE', array($this->controller, 'describe')); + $this->request->setMethod('DESCRIBE'); + $result = $this->controller->dispatch($this->request, $this->response); + $this->assertArrayHasKey('description', $result); + $this->assertContains('::describe', $result['description']); + } + public function testDispatchCallsActionMethodBasedOnNormalizingAction() { $this->routeMatch->setParam('action', 'test.some-strangely_separated.words'); diff --git a/tests/ZendTest/Mvc/Controller/TestAsset/Request.php b/tests/ZendTest/Mvc/Controller/TestAsset/Request.php new file mode 100644 index 00000000000..a4c31f34bb2 --- /dev/null +++ b/tests/ZendTest/Mvc/Controller/TestAsset/Request.php @@ -0,0 +1,33 @@ +method = $method; + return $this; + } +} diff --git a/tests/ZendTest/Mvc/Controller/TestAsset/RestfulTestController.php b/tests/ZendTest/Mvc/Controller/TestAsset/RestfulTestController.php index ce3fb5d4ea9..231ded99bd7 100644 --- a/tests/ZendTest/Mvc/Controller/TestAsset/RestfulTestController.php +++ b/tests/ZendTest/Mvc/Controller/TestAsset/RestfulTestController.php @@ -18,13 +18,26 @@ class RestfulTestController extends AbstractRestfulController public $entity = array(); /** - * Return list of resources + * Create a new resource * + * @param mixed $data * @return mixed */ - public function getList() + public function create($data) { - return array('entities' => $this->entities); + return array('entity' => $data); + } + + /** + * Delete an existing resource + * + * @param mixed $id + * @return mixed + */ + public function delete($id) + { + $this->entity = array(); + return array(); } /** @@ -39,39 +52,63 @@ public function get($id) } /** - * Create a new resource + * Return list of resources * - * @param mixed $data * @return mixed */ - public function create($data) + public function getList() { - return array('entity' => $data); + return array('entities' => $this->entities); } /** - * Update an existing resource + * Retrieve the headers for a given resource * - * @param mixed $id - * @param mixed $data - * @return mixed + * @return void */ - public function update($id, $data) + public function head($id = null) + { + } + + /** + * Return list of allowed HTTP methods + * + * @return \Zend\Http\Response + */ + public function options() + { + $response = $this->getResponse(); + $headers = $response->getHeaders(); + $headers->addHeaderLine('Allow', 'GET, POST, PUT, DELETE, PATCH, HEAD, TRACE'); + return $response; + } + + /** + * Patch (partial update) an entity + * + * @param int $id + * @param array $data + * @return array + */ + public function patch($id, $data) { + $entity = (array) $this->entity; $data['id'] = $id; - return array('entity' => $data); + $updated = array_merge($entity, $data); + return array('entity' => $updated); } /** - * Delete an existing resource + * Update an existing resource * * @param mixed $id + * @param mixed $data * @return mixed */ - public function delete($id) + public function update($id, $data) { - $this->entity = array(); - return array(); + $data['id'] = $id; + return array('entity' => $data); } public function editAction() @@ -83,4 +120,9 @@ public function testSomeStrangelySeparatedWordsAction() { return array('content' => 'Test Some Strangely Separated Words'); } + + public function describe() + { + return array('description' => __METHOD__); + } } From dc8dc313326cac79c4889d4f35549d16789aec9c Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Fri, 4 Jan 2013 14:46:21 -0600 Subject: [PATCH 2/2] Remove abstract keyword from new methods - Cannot add new abstract methods (BC break); instead, raise exceptions if unimplemented. - Added in missing patch() method. --- .../Controller/AbstractRestfulController.php | 42 +++++++++++++++++-- 1 file changed, 38 insertions(+), 4 deletions(-) diff --git a/library/Zend/Mvc/Controller/AbstractRestfulController.php b/library/Zend/Mvc/Controller/AbstractRestfulController.php index d15c001698c..cbde84af11b 100644 --- a/library/Zend/Mvc/Controller/AbstractRestfulController.php +++ b/library/Zend/Mvc/Controller/AbstractRestfulController.php @@ -74,13 +74,29 @@ abstract public function delete($id); */ abstract public function get($id); + /** + * Return list of resources + * + * @return mixed + */ + abstract public function getList(); + /** * Retrieve HEAD metadata for the resource * + * Not marked as abstract, as that would introduce a BC break + * (introduced in 2.1.0); instead, raises an exception if not implemented. + * * @param null|mixed $id * @return mixed + * @throws Exception\RuntimeException */ - abstract public function head($id = null); + public function head($id = null) + { + throw new Exception\RuntimeException(sprintf( + '%s is unimplemented', __METHOD__ + )); + } /** * Respond to the OPTIONS method @@ -88,16 +104,34 @@ abstract public function head($id = null); * Typically, set the Allow header with allowed HTTP methods, and * return the response. * + * Not marked as abstract, as that would introduce a BC break + * (introduced in 2.1.0); instead, raises an exception if not implemented. + * * @return mixed + * @throws Exception\RuntimeException */ - abstract public function options(); + public function options() + { + throw new Exception\RuntimeException(sprintf( + '%s is unimplemented', __METHOD__ + )); + } /** - * Return list of resources + * Respond to the PATCH method + * + * Not marked as abstract, as that would introduce a BC break + * (introduced in 2.1.0); instead, raises an exception if not implemented. * * @return mixed + * @throws Exception\RuntimeException */ - abstract public function getList(); + public function patch($id, $data) + { + throw new Exception\RuntimeException(sprintf( + '%s is unimplemented', __METHOD__ + )); + } /** * Update an existing resource