[HttpFoundation] [WIP] rename `Request::$request` to `$post` #47938

kbond · 2022-10-20T17:51:36Z

Q	A
Branch?	6.3
Bug fix?	no
New feature?	yes
Deprecations?	yes
Tickets	#47646
License	MIT
Doc PR	todo

This should alleviate the confusion of what $request->request is.

TODO:

deprecation layer(?)
update changelog

kbond · 2022-10-20T17:52:31Z

I could use some advise on how to create a deprecation layer for this. To avoid a BC break, Request::$request and Request::$form must reference the same InputBag instance.

Also, since $request/$form are public and writable, you can change one without the other...

Could we use __set()/__get() to provide a deprecation layer?

kbond · 2022-10-20T21:08:15Z

FYI, I chose $form because this is what $_POST represents, the form data. I'm not completely convinced that $form is the best term (it's better than $request IMO) - maybe $post or $postData would be better?

welcoMattic · 2022-10-21T07:31:45Z

src/Symfony/Component/HttpFoundation/Request.php

+     *
+     * @var InputBag
+     */
+    public $form;


What about naming it $postData?

the word data is implicit via the $request object no?
a request (class) transferring a data via http (foundation)
to me $post is good candidate :)

stof · 2022-10-21T09:10:34Z

To me, $request->form is an equally bad name. <form method="get"> would not put its data in the form property

94noni · 2022-10-21T09:17:43Z

@kbond you can organize a pool on slack or here to get some community feedbacks on "proper" new naming
I like $post for representing $_post

stof · 2022-10-21T10:36:10Z

src/Symfony/Component/HttpFoundation/Request.php

@@ -89,10 +89,19 @@ class Request
    /**
     * Request body parameters ($_POST).
     *
+     * @deprecated since Symfony 6.2
+     *
     * @var InputBag
     */
    public $request;


this should probably be implemented through __get and __set, to avoid issue where someone re-assign the property, where we would have different instances in the new property and the old one (breaking the BC layer)

alexander-schranz · 2022-10-21T11:27:57Z

Personally don't think its worth renaming this. This will be a hard break for a lot of Systems, which tries in the past supporting multiple Symfony versions.

And if I would more go with something based on the http specification. I think there it is also sometimes represented as request, but also as Content / Message or Body. For me form is more confusing then request as a form is GET or POST in HTML and the request data is more for PUT, DELETE, PATCH, ...

In PSR7 it is referenced as getParsedBody so I think body sould reference best, else the same problem I think will exist with other name if we don't make it similar to other existing standard.

kbond · 2022-10-22T19:52:53Z

Personally don't think its worth renaming this. This will be a hard break for a lot of Systems, which tries in the past supporting multiple Symfony versions.

This is fair. Maybe we could have a longer deprecation cycle? Remove $request in Symfony 8?

Seems like $post would be a good candidate and I agree.

wouterj · 2022-10-22T22:24:44Z

My 2-cents:

What I liked about the original proposal in #47646 is the fact that we don't have to deprecate $request->request. We add a new property (name tbd), which covers the functionality of $request->request on top of extended functionality (parsing more request body content types, whereas $request->request only "parses" application/x-www-form-urlencoded related content types).

This means we don't deprecate $request->request (i.e. no impact on existing apps), but we update the docs to use the new property instead.

Duplicating what I said on Slack: the HTTP spec calls this "request payload body", maybe $request->payload is a nice name?

GromNaN · 2022-10-27T10:21:16Z

👍🏻 for naming $request->payload. This is the name in the RFC 7231 and Chrome Dev Tools.
👍🏻 for keeping the existing $request->request untouched for BC. The new field will have a new behavior.

Additionally, $request->payload should be lazy-loaded: JSON decode should be done only when requested and throw BadRequestException in case of invalid format.

kbond · 2022-12-07T14:36:23Z

Ok, I'm going to update this PR to just add Request::$payload. Instead of a new property, what about a Request::getPayload() method instead? This could avoid the need for a LazyParameterBag.

luxferoo · 2023-01-25T14:48:28Z

@kbond Thank you for the great work ! When do you think it would be available please ?

kbond · 2023-01-25T14:58:19Z

I'm hoping to get rework this soon and try and get it into 6.3.

nicolas-grekas · 2023-03-09T15:07:19Z

closing in favor of #49614

This PR was merged into the 6.3 branch. Discussion ---------- [HttpFoundation] add `Request::getPayload()` | Q | A | ------------- | --- | Branch? | 6.3 | Bug fix? | no | New feature? | yes | Deprecations? | no | Tickets | Fix #47646 | License | MIT | Doc PR | todo Alternative for #47938 based on feedback there. ```php /** `@var` InputBag $input wrapping either Request::$request or Request::toArray() */ $input = $request->getPayload(); ``` Commits ------- 4a0a439 [HttpFoundation] add `Request::getPayload()`

[HttpFoundation] rename Request::$request to $form

cc92de5

carsonbot added Status: Needs Work Bug Deprecation Feature HttpFoundation labels Oct 20, 2022

carsonbot added this to the 6.2 milestone Oct 20, 2022

carsonbot changed the title ~~[WIP][HttpFoundation] rename Request::$request to $form~~ [HttpFoundation] [WIP] rename Request::$request to $form Oct 20, 2022

kbond removed the Bug label Oct 20, 2022

kbond mentioned this pull request Oct 20, 2022

[RFC] $request->body->get #47646

Closed

kbond modified the milestones: 6.2, 6.3 Oct 20, 2022

welcoMattic reviewed Oct 21, 2022

View reviewed changes

stof requested changes Oct 21, 2022

View reviewed changes

kbond changed the title ~~[HttpFoundation] [WIP] rename Request::$request to $form~~ [HttpFoundation] [WIP] rename Request::$request to $post Oct 22, 2022

kbond added the RFC RFC = Request For Comments (proposals about features that you want to be discussed) label Oct 22, 2022

wouterj mentioned this pull request Jan 6, 2023

Allow Remember Me cookie with JSON Login #40918

Closed

GromNaN mentioned this pull request Jan 9, 2023

[HttpFoundation] setContent missing in Request class #48357

Closed

kbond mentioned this pull request Mar 6, 2023

[HttpFoundation] add Request::getPayload() #49614

Merged

nicolas-grekas closed this Mar 9, 2023

Uh oh!

[HttpFoundation] [WIP] rename Request::$request to $post #47938

[HttpFoundation] [WIP] rename Request::$request to $post #47938

Uh oh!

Conversation

kbond commented Oct 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kbond commented Oct 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kbond commented Oct 20, 2022

Uh oh!

welcoMattic Oct 21, 2022

Choose a reason for hiding this comment

Uh oh!

94noni Oct 21, 2022

Choose a reason for hiding this comment

Uh oh!

stof commented Oct 21, 2022

Uh oh!

94noni commented Oct 21, 2022

Uh oh!

stof Oct 21, 2022

Choose a reason for hiding this comment

Uh oh!

alexander-schranz commented Oct 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kbond commented Oct 22, 2022

Uh oh!

wouterj commented Oct 22, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

GromNaN commented Oct 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kbond commented Dec 7, 2022

Uh oh!

luxferoo commented Jan 25, 2023

Uh oh!

kbond commented Jan 25, 2023

Uh oh!

nicolas-grekas commented Mar 9, 2023

Uh oh!

Uh oh!

[HttpFoundation] [WIP] rename `Request::$request` to `$post` #47938

[HttpFoundation] [WIP] rename `Request::$request` to `$post` #47938

kbond commented Oct 20, 2022 •

edited

Loading

kbond commented Oct 20, 2022 •

edited

Loading

alexander-schranz commented Oct 21, 2022 •

edited

Loading

wouterj commented Oct 22, 2022 •

edited

Loading

GromNaN commented Oct 27, 2022 •

edited

Loading