Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Add get decode json data on params controller plugin #5897

Closed
wants to merge 4 commits into from

4 participants

@lies

No description provided.

@Ocramius Ocramius added Mvc Json labels
@Ocramius Ocramius added this to the 2.3.0 milestone
@EvanDotPro EvanDotPro was assigned by Ocramius
library/Zend/Mvc/Controller/Plugin/Params.php
@@ -117,4 +118,34 @@ public function fromRoute($param = null, $default = null)
return $controller->getEvent()->getRouteMatch()->getParam($param, $default);
}
+
+ /**
+ * Return all decode json parameters or a single decode json parameter
+ *
+ * @param string $param Parameter name to retrieve, or null to get all.
+ * @param mixed $default Default value to use when the parameter is missing.
+ * @return mixed
+ */
+ public function fromJsonRawBody($param = null, $default = null)
+ {
+ if (empty($this->jsonDecoded)) {
@Ocramius Collaborator
Ocramius added a note

@EvanDotPro you suggested caching the decoded json - is it such a big deal here?

I see more problems than anything coming from this, since the plugin is now unusable over multiple requests and/or internal forwards.

@Ocramius Collaborator
Ocramius added a note

Yeah, after re-reading the method body, caching the decoded json is an overkill here. I'd just suggest getting rid of this.

Sorry about that, @lies

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
library/Zend/Mvc/Controller/Plugin/Params.php
((8 lines not shown))
+ * @param string $param Parameter name to retrieve, or null to get all.
+ * @param mixed $default Default value to use when the parameter is missing.
+ * @return mixed
+ */
+ public function fromJsonRawBody($param = null, $default = null)
+ {
+ if (empty($this->jsonDecoded)) {
+ $content = $this->getController()->getRequest()->getContent();
+
+ if (empty($content)) {
+ return $default;
+ }
+
+ try{
+ $this->jsonDecoded = Json::decode($content, Json::TYPE_ARRAY);
+ }catch (\Exception $e){
@Ocramius Collaborator
Ocramius added a note

Is there a Json specific exception? I'm thinking about catching Zend\Json\Exception\ExceptionInterface.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
library/Zend/Mvc/Controller/Plugin/Params.php
((9 lines not shown))
+ * @param mixed $default Default value to use when the parameter is missing.
+ * @return mixed
+ */
+ public function fromJsonRawBody($param = null, $default = null)
+ {
+ if (empty($this->jsonDecoded)) {
+ $content = $this->getController()->getRequest()->getContent();
+
+ if (empty($content)) {
+ return $default;
+ }
+
+ try{
+ $this->jsonDecoded = Json::decode($content, Json::TYPE_ARRAY);
+ }catch (\Exception $e){
+ $this->jsonDecoded = array();
@Ocramius Collaborator
Ocramius added a note

If something went wrong, then the decoded variable should probably be null

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
library/Zend/Mvc/Controller/Plugin/Params.php
((13 lines not shown))
+ {
+ if (empty($this->jsonDecoded)) {
+ $content = $this->getController()->getRequest()->getContent();
+
+ if (empty($content)) {
+ return $default;
+ }
+
+ try{
+ $this->jsonDecoded = Json::decode($content, Json::TYPE_ARRAY);
+ }catch (\Exception $e){
+ $this->jsonDecoded = array();
+ }
+ }
+
+ if ($param != null) {
@Ocramius Collaborator
Ocramius added a note

Should be a strict comparison

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
library/Zend/Mvc/Controller/Plugin/Params.php
((14 lines not shown))
+ if (empty($this->jsonDecoded)) {
+ $content = $this->getController()->getRequest()->getContent();
+
+ if (empty($content)) {
+ return $default;
+ }
+
+ try{
+ $this->jsonDecoded = Json::decode($content, Json::TYPE_ARRAY);
+ }catch (\Exception $e){
+ $this->jsonDecoded = array();
+ }
+ }
+
+ if ($param != null) {
+ return array_key_exists($param, $this->jsonDecoded) ? $this->jsonDecoded[$param] : $default;
@Ocramius Collaborator
Ocramius added a note

This assumes the decoded json is an array - the previous check should therefore use is_array()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@Ocramius
Collaborator

@lies for further changes, just push new commits to this branch - that will automatically update the pull request

@weierophinney

I'm going to say "no" to this PR. The reason is that we really shouldn't be pushing this sort of functionality directly into the controllers; I'd actually prefer to be removing functionality from the base controllers at this point. Functionality like this should be via controller plugins. We've actually implemented this exact functionality already via zfcampus/zf-content-negotiation, in the bodyParams plugin. This allows you to do the following:

$params = $this->bodyParams();

or, to retrieve a single parameter:

$param = $this->bodyParam('name', 'defaultValue');

An approach like this keeps the ZF2 MVC lighter, makes the functionality opt-in, and, in this case, provides more possibilities for extension to support a larger number of mediatypes.

@lies

:)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
30 library/Zend/Mvc/Controller/Plugin/Params.php
@@ -9,6 +9,8 @@
namespace Zend\Mvc\Controller\Plugin;
+use Zend\Json\Exception\ExceptionInterface;
+use Zend\Json\Json;
use Zend\Mvc\Exception\RuntimeException;
use Zend\Mvc\InjectApplicationEventInterface;
@@ -117,4 +119,32 @@ public function fromRoute($param = null, $default = null)
return $controller->getEvent()->getRouteMatch()->getParam($param, $default);
}
+
+ /**
+ * Return all decode json parameters or a single decode json parameter
+ *
+ * @param string $param Parameter name to retrieve, or null to get all.
+ * @param mixed $default Default value to use when the parameter is missing.
+ * @return mixed
+ */
+ public function fromJsonRawBody($param = null, $default = null)
+ {
+ $content = $this->getController()->getRequest()->getContent();
+
+ if(empty($content)){
+ return null;
+ }
+
+ try{
+ $json = Json::decode($content, Json::TYPE_ARRAY);
+ }catch (ExceptionInterface $e){
+ return null;
+ }
+
+ if($param !== null && is_array($json)){
+ return array_key_exists($param, $json) ? $json[$param] : $default;
+ }
+
+ return $json;
+ }
}
View
54 tests/ZendTest/Mvc/Controller/Plugin/ParamsTest.php
@@ -201,6 +201,53 @@ public function testInvokeWithNoArgumentsReturnsInstance()
$this->assertSame($this->plugin, $this->plugin->__invoke());
}
+ public function testFromJsonRawBodyReturnsDefaultIfSet()
+ {
+ $this->setJson();
+
+ $value = $this->plugin->fromJsonRawBody('foo', 'bar');
+
+ $this->assertEquals($value, 'bar');
+ }
+
+ public function testFromJsonRawBodyReturnDefaultIfContentNotSet()
+ {
+ $value = $this->plugin->fromJsonRawBody();
+
+ $this->assertEquals($value, null);
+ }
+
+ public function testFromJsonRawBodyReturnEmptyArrayIfInvalidJsonOnBody()
+ {
+ $this->request->setContent("foo => bar");
+ $this->controller->dispatch($this->request);
+
+ $values = $this->plugin->fromJsonRawBody();
+
+ $this->assertEquals($values, null);
+ }
+
+ public function testFromJsonRawBodyReturnsExceptedValue()
+ {
+ $this->setJson();
+
+ $value = $this->plugin->fromJsonRawBody('value', 'default');
+ $valueOther = $this->plugin->fromJsonRawBody('other', 'default');
+
+ $this->assertEquals($value, 'json:1234');
+ $this->assertEquals($valueOther, '1234:other');
+ }
+
+ public function testFromJsonRawBodyReturnsAll()
+ {
+ $this->setJson();
+
+ $values = $this->plugin->fromJsonRawBody();
+
+ $this->assertEquals(2, count($values));
+ $this->assertEquals($values, array('value' => 'json:1234', 'other' => '1234:other'));
+ }
+
protected function setQuery()
{
$this->request->setMethod(Request::METHOD_GET);
@@ -218,4 +265,11 @@ protected function setPost()
$this->controller->dispatch($this->request);
}
+
+ protected function setJson()
+ {
+ $this->request->setContent(json_encode(array('value' => 'json:1234', 'other' => '1234:other')));
+
+ $this->controller->dispatch($this->request);
+ }
}
Something went wrong with that request. Please try again.