Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Added Json support for POST and PUT operations in restful controller. #3028

Closed
wants to merge 6 commits into from

2 participants

Jurgen Van de Moere Matthew Weier O'Phinney
Jurgen Van de Moere

Added Json support for POST and PUT operations in restful controllers.

This is particularly handy if you are using a javascript library (such as AngularJS by Google) that uses "Content-Type: application/json" for POST and PUT operations.

This PR detects if the content type is set to json (in request headers) and makes sure that the "create" and "update" methods of the restful controller receive the correct data.

Without this PR, the data is empty and one needs to decode the response body manually.

Change-Id: I11963778fa99d1dd1c523203c25d5a9f53958310

Jurgen Van de Moere Added Json support for POST and PUT operations in restful controller.
Change-Id: I11963778fa99d1dd1c523203c25d5a9f53958310
50564ab
...ary/Zend/Mvc/Controller/AbstractRestfulController.php
@@ -11,6 +11,7 @@
namespace Zend\Mvc\Controller;
use Zend\Http\Request as HttpRequest;
+use Zend\Json\Decoder;
Matthew Weier O'Phinney Owner

Don't use Zend\Json\Decoder. Use Zend\Json\Json::decode() instead, as it uses json_decode() by default. Decoder should only be used in rare occasions, typically when the JSON is known to be in an encoding other than UTF-8, or where you know JS lambdas may be included in the content.

Could you be more specific ?

Do you suggest to have Zend\Json\Decover replaced with json_decode ?

Or do you want to skip automatic decoding in the abstract class and leave decoding to the controller instance ?

Matthew Weier O'Phinney Owner

I mean:

  • import Zend\Json\Json instead of Zend\Json\Decoder
  • Use Json::decode() instead of Decoder::decode()

In other words, continue with automated decoding, just change the mechanism. :)

Ok, got it, I will update the code and notify you when finished. Thanks !!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
...ary/Zend/Mvc/Controller/AbstractRestfulController.php
((6 lines not shown))
{
+ $contentType = strtolower( $request->getHeaders()->get('Content-Type')->getFieldValue());
+
+ if (strpos($contentType, 'application/json') !== false) {
Matthew Weier O'Phinney Owner

I'm wondering if this check should be configurable. It's not uncommon to define custom media types such as application/vnd.my.app+json -- which would fail this check, but still be something you'd want to automatically decode. The configuration could be present in a protected class member.

Very true, do you think it would be useful to handle other content types as well ?

If so, maybe we can create some kind of pluggable content type handler that formats data depending on content type...

Matthew Weier O'Phinney Owner

You might take a look at how the new acceptedViewModel plugin works -- you pass it a list of Accept media types you want to match. We could do similarly here, but instead have those in a protected member so that developers could specify them themselves:

class MyResource extends AbstractRestfulController
{
    protected $jsonContentTypes = array(
        'application/vnd.myapp.resource+json',
        'application/json',
    );
}

The code would then check the Content-Type header value against each of these; if any match, it does the decoding.

Good idea, I will implement it like that and get back to you when ready. Thanks !!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Jurgen Van d... added some commits
Jurgen Van de Moere Replaced Zend\Json\Decoder::decode with Zend\Json\Json::decode
Change-Id: Iac885dbf1511dfb8087d7f3586ae845e071b26c5
48addfa
Jurgen Van de Moere Added protected contentTypes array and requestHasContentType function
Change-Id: I222617c77b3942fdc5b8263b827c1c58dab570b7
9dfdefb
Jurgen Van de Moere

Hi @weierophinney,

I updated the code with a generic content type checker that can later be extended for other content types as well (if necessary).

The protected contentTypes array can now easily be overridden to add more content type values to check against.

PS: I noticed some strange test file updates in the commit that were pushed by my zend studio...

...ary/Zend/Mvc/Controller/AbstractRestfulController.php
((6 lines not shown))
/**
* @var string
*/
protected $eventIdentifier = __CLASS__;
-
+
+ /**
+ * @var array
+ */
+ protected $contentTypes = array(
+ self::CONTENT_TYPE_JSON => array(
+ 'application/vnd.myapp.resource+json',
Matthew Weier O'Phinney Owner

I'd remove this one. However, I'd likely add "application/hal+json" to it (as that's becoming a common standard).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
...ary/Zend/Mvc/Controller/AbstractRestfulController.php
((9 lines not shown))
$content = $request->getContent();
parse_str($content, $parsedParams);
return $this->update($id, $parsedParams);
}
+
+ /**
+ * Check if request has certain content type
+ *
+ * @return boolean
+ */
+ public function requestHasContentType (Request $request, $contentType = '')
Matthew Weier O'Phinney Owner

Use the Accept header object's match() method instead of what you do in here; it's more robust, and won't return a false positive in situations where other content types have higher priority.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Matthew Weier O'Phinney

Looks like the various files under _files should not have been changed; can you remove those from the PR, by any chance? Also, please address the other feedback I provided.

Otherwise... looking good!

Matthew Weier O'Phinney

@jvandemo Will you be able to get this ready in the next 1-2 weeks? If not, I need to change the milestone.

Jurgen Van de Moere

@weierophinney Yes, absolutely. I just got back today from a 2 week absence due to the birth of my son, so I'll fix it asap. Thanks !!

Jurgen Van d... added some commits
Jurgen Van de Moere Revert "Added protected contentTypes array and requestHasContentType …
…function"

This reverts commit 9dfdefbf4c9d695bc0d2125c30571ae37ff95a89.
66ffac0
Jurgen Van de Moere Updated requestHasContentType to use the match method of the Accept
header object.

Change-Id: I7e43851c88692b5ea675393719f91d3a4ef379dd
b69296a
Jurgen Van de Moere Format source code
Change-Id: I8035798710e17d3e0ec2fdf08fe2d0e1b8086732
3557b13
Jurgen Van de Moere

@weierophinney The various files under _files were removed and the changes you suggested have been implemented.

Let me know if you need any other changes.

Thanks !

Matthew Weier O'Phinney weierophinney referenced this pull request from a commit
Matthew Weier O'Phinney weierophinney [#3028] Add check for existence of Accept header
- If we don't have an Accept header, we can't test for allowed content type, and
  must return false
e353a79
Matthew Weier O'Phinney

Merged to develop branch for 2.1 release -- thanks!

katalonec katalonec referenced this pull request from a commit in katalonec/zf2
Matthew Weier O'Phinney weierophinney Merge branch 'feature/3028' into develop
Close #3028
f3b4e2b
Deleted user Unknown referenced this pull request from a commit
Matthew Weier O'Phinney weierophinney [#3028] Add check for existence of Accept header
- If we don't have an Accept header, we can't test for allowed content type, and
  must return false
4833554
Deleted user Unknown referenced this pull request from a commit
Matthew Weier O'Phinney weierophinney Merge branch 'feature/3028' into develop
Close #3028
c0a33e6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Nov 20, 2012
  1. Added Json support for POST and PUT operations in restful controller.

    Jurgen Van de Moere authored
    Change-Id: I11963778fa99d1dd1c523203c25d5a9f53958310
Commits on Nov 21, 2012
  1. Replaced Zend\Json\Decoder::decode with Zend\Json\Json::decode

    Jurgen Van de Moere authored
    Change-Id: Iac885dbf1511dfb8087d7f3586ae845e071b26c5
  2. Added protected contentTypes array and requestHasContentType function

    Jurgen Van de Moere authored
    Change-Id: I222617c77b3942fdc5b8263b827c1c58dab570b7
Commits on Jan 3, 2013
  1. Revert "Added protected contentTypes array and requestHasContentType …

    Jurgen Van de Moere authored
    …function"
    
    This reverts commit 9dfdefbf4c9d695bc0d2125c30571ae37ff95a89.
  2. Updated requestHasContentType to use the match method of the Accept

    Jurgen Van de Moere authored
    header object.
    
    Change-Id: I7e43851c88692b5ea675393719f91d3a4ef379dd
  3. Format source code

    Jurgen Van de Moere authored
    Change-Id: I8035798710e17d3e0ec2fdf08fe2d0e1b8086732
This page is out of date. Refresh to see the latest.
Showing with 66 additions and 19 deletions.
  1. +66 −19 library/Zend/Mvc/Controller/AbstractRestfulController.php
85 library/Zend/Mvc/Controller/AbstractRestfulController.php
View
@@ -7,10 +7,10 @@
* @license http://framework.zend.com/license/new-bsd New BSD License
* @package Zend_Mvc
*/
-
namespace Zend\Mvc\Controller;
use Zend\Http\Request as HttpRequest;
+use Zend\Json\Json;
use Zend\Mvc\Exception;
use Zend\Mvc\MvcEvent;
use Zend\Stdlib\RequestInterface as Request;
@@ -25,12 +25,25 @@
*/
abstract class AbstractRestfulController extends AbstractController
{
+
+ const CONTENT_TYPE_JSON = 'json';
+
/**
* @var string
*/
protected $eventIdentifier = __CLASS__;
/**
+ * @var array
+ */
+ protected $contentTypes = array(
+ self::CONTENT_TYPE_JSON => array(
+ 'application/application/hal+json',
+ 'application/json'
+ )
+ );
+
+ /**
* Return list of resources
*
* @return mixed
@@ -78,8 +91,10 @@
public function notFoundAction()
{
$this->response->setStatusCode(404);
-
- return array('content' => 'Page not found');
+
+ return array(
+ 'content' => 'Page not found'
+ );
}
/**
@@ -97,10 +112,11 @@ public function notFoundAction()
*/
public function dispatch(Request $request, Response $response = null)
{
- if (!$request instanceof HttpRequest) {
- throw new Exception\InvalidArgumentException('Expected an HTTP request');
+ if (! $request instanceof HttpRequest) {
+ throw new Exception\InvalidArgumentException(
+ 'Expected an HTTP request');
}
-
+
return parent::dispatch($request, $response);
}
@@ -114,20 +130,21 @@ public function dispatch(Request $request, Response $response = null)
public function onDispatch(MvcEvent $e)
{
$routeMatch = $e->getRouteMatch();
- if (!$routeMatch) {
+ if (! $routeMatch) {
/**
* @todo Determine requirements for when route match is missing.
* Potentially allow pulling directly from request metadata?
*/
- throw new Exception\DomainException('Missing route matches; unsure how to retrieve action');
+ throw new Exception\DomainException(
+ 'Missing route matches; unsure how to retrieve action');
}
-
+
$request = $e->getRequest();
- $action = $routeMatch->getParam('action', false);
+ $action = $routeMatch->getParam('action', false);
if ($action) {
// Handle arbitrary methods, ending in Action
$method = static::getMethodFromAction($action);
- if (!method_exists($this, $method)) {
+ if (! method_exists($this, $method)) {
$method = 'notFoundAction';
}
$return = $this->$method();
@@ -158,8 +175,9 @@ public function onDispatch(MvcEvent $e)
break;
case 'delete':
if (null === $id = $routeMatch->getParam('id')) {
- if (!($id = $request->getQuery()->get('id', false))) {
- throw new Exception\DomainException('Missing identifier');
+ if (! ($id = $request->getQuery()->get('id', false))) {
+ throw new Exception\DomainException(
+ 'Missing identifier');
}
}
$action = 'delete';
@@ -168,15 +186,15 @@ public function onDispatch(MvcEvent $e)
default:
throw new Exception\DomainException('Invalid HTTP method!');
}
-
+
$routeMatch->setParam('action', $action);
}
-
+
// Emit post-dispatch signal, passing:
// - return from method, request, response
// If a listener returns a response object, return it immediately
$e->setResult($return);
-
+
return $return;
}
@@ -188,7 +206,12 @@ public function onDispatch(MvcEvent $e)
*/
public function processPostData(Request $request)
{
- return $this->create($request->getPost()->toArray());
+ if ($this->requestHasContentType($request, self::CONTENT_TYPE_JSON)) {
+ return $this->create(Json::decode($request->getContent()));
+ }
+
+ return $this->create($request->getPost()
+ ->toArray());
}
/**
@@ -202,14 +225,38 @@ public function processPostData(Request $request)
public function processPutData(Request $request, $routeMatch)
{
if (null === $id = $routeMatch->getParam('id')) {
- if (!($id = $request->getQuery()->get('id', false))) {
+ if (! ($id = $request->getQuery()->get('id', false))) {
throw new Exception\DomainException('Missing identifier');
}
}
+
+ if ($this->requestHasContentType($request, self::CONTENT_TYPE_JSON)) {
+ return $this->update($id, Json::decode($request->getContent()));
+ }
+
$content = $request->getContent();
parse_str($content, $parsedParams);
-
+
return $this->update($id, $parsedParams);
}
+ /**
+ * Check if request has certain content type
+ *
+ * @return boolean
+ */
+ public function requestHasContentType(Request $request, $contentType = '')
+ {
+ $acceptHeaders = $request->getHeaders()->get('Accept');
+
+ if (array_key_exists($contentType, $this->contentTypes)) {
+ foreach ($this->contentTypes[$contentType] as $contentTypeValue) {
+ if ($acceptHeaders->match($contentTypeValue) !== false) {
+ return true;
+ }
+ }
+ }
+
+ return false;
+ }
}
Something went wrong with that request. Please try again.