-
Notifications
You must be signed in to change notification settings - Fork 150
JsonResponse additional API #250
JsonResponse additional API #250
Conversation
- getEncodingOptions(): int - getPayload(): mixed - withEncodingOptions(int $encodingOptions) - withPayload($data)
178e988
to
b1208ce
Compare
* | ||
* @return JsonResponse | ||
*/ | ||
public function withPayload($data) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To create new object with already encoded payload try to use named constructors 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As written, this follows the design of other value objects in this repository, making usage predictable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good. I have one suggestion for improvement, which I will take care of during merge.
* | ||
* @return JsonResponse | ||
*/ | ||
public function withPayload($data) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As written, this follows the design of other value objects in this repository, making usage predictable.
|
||
$json = $this->jsonEncode($new->payload, $new->encodingOptions); | ||
$body = $this->createBodyFromJson($json); | ||
$new = $new->withBody($body); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd extract these three lines into a separate method, in order to prevent duplication. Something along the lines of:
private function updateBodyFor(self $toUpdate)
{
$json = $this->jsonEncode($toUpdate->payload, $toUpdate->encodingOptions);
$body = $this->createBodyFromJson($json);
return $toUpdate->withBody($body);
}
Then, update this method and others like it so that they read:
return $this->updateBodyFor($new);
JsonResponse additional API Conflicts: CHANGELOG.md
Thanks, @stefanotorresi. Merged to develop for release with 2.5.0. |
This PR adds new API to
Zend\Diactoros\Response\JsonResponse
:withPayload($data)
getPayload()
withEncodingOptions($options)
getEncodingOptions()
The constructor now holds a copy of the payload and its encoding options to let consumer modify the final content without having to get the body content, decode it, update it, and then re-invoke the full constructor.
The main use case that inspired this new functionality is adding metadata to a JSON down a middleware pipe, e.g. hypermedia links.
While the native PHP JSON parser is fast enough for simple use cases, this can become a problem with high loads and big JSONs; besides, what consumers must resolve to do is arguably in-elegant; here is an example of a middleware that adds a
self
link to a JSON response:The initial idea was to avoid encoding the data in the constructor, but rather do it lazily in the
getBody
method; unfortunately, this would be backwards incompatible due to the validation of the payload happening in__construct()
. This makes necessary to encode the payload as soon as the response is created, but at least now the decoding step can be avoided by getting the payload from the response and creating a new instance by invokingwithPayload($data)
;So the new API enables to change the previous middlware example to this:
It may not seem like a big deal, but I think it is overall less wonky! 😉