[HttpFoundation] add Request::getPayload()
#49614
Conversation
f1c6ce6 to
68ec79e
Compare
|
are we sure about getPayload vs getBody:
|
Yes. The HTTP RFC is a better reference than ChatGPT. See #47938 (comment) |
|
well, the RFC does mention "payload header fields" vs. "payload body" 😅 i tend to agree |
68ec79e to
b8c5ef6
Compare
There was a problem hiding this comment.
I updated the PR to always return a new InputBag instance, and I added a cache for the decoded content.
This means this feature wouldn't allow mutating the request payload.
This solves synchronization issues that were spotted during the review, at the cost of not providing the DX you're looking for @kbond maybe?
I don't think we can nor should work around the synchronization issues at the request level, so I'd be fine with this updated implementation, if it provides enough value. (let's close if not)
| /** | ||
| * Gets the request body decoded as array, typically from a JSON payload. | ||
| * | ||
| * @throws JsonException When the body cannot be decoded to an array | ||
| */ | ||
| public function toArray(): array | ||
| { | ||
| if (null !== $this->arrayContent) { |
There was a problem hiding this comment.
IIUC it could be spared out by doing things in reverse, thus return $this->getPayload()->all() here ..
then i have to ask, how useful is this method still? Since it's not a request-array representation, but merely a content one
There was a problem hiding this comment.
Agree this method is a bit superfluous now. I guess if the request has a json body and $_POST parameters it could still be useful to get the json body? I don't know if that's a realistic scenario.
There was a problem hiding this comment.
Deprecations come at a cost for the community, there is no need to remove this method IMHO.
There was a problem hiding this comment.
Think $_POST and JSON body combined is not possible via a real HTTP call? As $_POST is using the body to provide the data and so the body can not be used for json?
There was a problem hiding this comment.
Think $_POST and JSON body combined is not possible via a real HTTP call? As $_POST is using the body to provide the data and so the body can not be used for json?
If that's true than the method is indeed superfluous. But, I agree with Nicolas, let's leave that to another PR.
b8c5ef6 to
4a0a439
Compare
|
Thank you @kbond. |
This PR was merged into the 6.3 branch. Discussion ---------- [HttpFoundation] Add note on Request::getPayload() | Q | A | ------------- | --- | Branch? |  | Bug fix? | no | New feature? | no | Deprecations? | no | Tickets | Fix #... <!-- prefix each issue number with "Fix #", no need to create an issue if none exists, explain below instead --> | License | MIT | Doc PR | symfony/symfony-docs#... <!-- required for new features --> <!-- Replace this notice by a short README for your feature/bugfix. This will help reviewers and should be a good start for the documentation. Additionally (see https://symfony.com/releases): - Always add tests and ensure they pass. - Bug fixes must be submitted against the lowest maintained branch where they apply (lowest branches are regularly merged to upper ones so they get the fixes too). - Features and deprecations must be submitted against the latest branch. - For new features, provide some code snippets to help understand usage. - Changelog entry should follow https://symfony.com/doc/current/contributing/code/conventions.html#writing-a-changelog-entry - Never break backward compatibility (see https://symfony.com/bc). --> i'd like to add a note on newly added getPayload in #49614 i hope this helps migrating to uniform access across the board, so we can deprecate these + get() somewhere in the future Commits ------- c97a551 Update Request.php
…t->getPayload()` (MrYamous) This PR was merged into the 6.4 branch. Discussion ---------- [HttpFoundation] replace `$request->request` by `$request->getPayload()` Considering new function getPayload added in [code #49614](symfony/symfony#49614), what do you think about replace `$request->request` by `$request->getPayload` in documentation ? Commits ------- 00e9b45 [HttpFoundation] replace $request->request by getPayload()
Alternative for #47938 based on feedback there.