-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[HttpFoundation] Allow Request#request to be ParameterBag #44789
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
I'm not really convinced we need this. What's the real motivation here? |
The main thing is when doing something like: $request->request = new ParameterBag(json_decode($request->getContent())); This has been the standard in FOSRestBundle since the start, making it a common practice in Symfony API implementations now (or let's talk only about what I know: we're still using this in our controllers as a leftover legacy from the original FOSRestBundle implementation we used to use). It's not so nice to use Thinking about it, I think there are 2 possibilities:
What do you think about (2)? (for 6.1 of course) |
i think semantically $request->request may fit any decoded payload. But sticking with form-urlencode, thus InputBag, now works for me. we could explore eg. dot-notation / property-access for InputBag |
Oh, didn't know about |
🤷 aiming for uniform acces thru either InputBag or toArray sounds noble. ->toArray = json is maybe weird |
InputBag|ParameterBag is blurring the line. toArray()has been added exactly for that use case (json) (I also forgot about it :X) |
Note SF just cant do it in a lazy manner (that is decoding when calling |
You folks need to now explain to all your users how to fix this problem your comments here thus far have been unclear. Consider that the way that in the npm/composer worlds, we now have third party code that is failing all over the place (and it is 16 months on.) That was a reckless change! |
@ddr5292 please open an issue if you believe anything's wrong still. |
Please document your changes and explain how to change our code so that we can use get past this error now. Give multiple examples for all use cases. |
@ddr5292 Please open an issue and explain your problem instead of ranting in the comments of PRs that have been closed for years. |
If you have a really big project using get() everywhere, then you know what problem it is. I'm facing the same issue after trying the upgrade and now started to doubt if it's the right thing to upgrade. we used get() for either string or array (json). And then we have logic in our code to make sure submitted value is in desired data type. (and I think it's programmers' responsibilities to validate data type before using instead of framework itself). The inputBag is doing too much job. With this breaking change, I need to review all get() and think about if that submitted value is supposed to be scalar or array. This process is tedious and error prone. I personally agree typed method is good for more predictable behavior. But the question is does it really worth introducing breaking change? why not keep get() as what it was and add getArray() for those who need typed data ? |
This ship sailed long ago. As you can see, this has been discussed numerous times 2 years ago, so it's unlikely that the outcome changes now. Furthermore, both deprecation and new behavior has been part of Symfony for nearly 4 years. Changing or reverting now is impossible and will impact more applications than changing this in existing applications that are upgrading to 5.x or 6.x now. Please also do not comment more or less the same things across multiple issues/PRs within the same topic. This divides the same discussion across multiple places, making it harder to track. |
I'm locking all of those issues now. If there's something NEW to discuss, please open a new issue. |
The InputBag story starts to walk circles, but I hope this is the last PR in the story.
$request->request
. [HttpFoundation] Add InputBag #34363 broke this use-case$request->request
can be allowed to beInputBag|ParameterBag
(by only usingInputBag
for form requests). Introduced by [HttpFoundation] use InputBag for Request::$request only if data is coming from a form #37265InputBag
was merged. While this fixesnull
and other scalar types, it doesn't fix the array type (which is perfectly fine if you're doing$request->request = new ParameterBag(json_decode(...));
)I think, to support putting JSON decode in
$request->request
, we must make it a union ofParameterBag|InputBag
.I'm not sure about the branch to target branch, feel free to change